Fix environment PATH pollution by CGrpc servers (#1590)

* Extract logic to get system's actual path

Signed-off-by: paul.profizi <[email protected]>

* Try commenting all path manipulations for CGrpc server in ServerFactory.get_server_type_from_config

Signed-off-by: paul.profizi <[email protected]>

* Add test on path pollution

Signed-off-by: paul.profizi <[email protected]>

* Make test retro-compatible (no server.release()

Signed-off-by: paul.profizi <[email protected]>

* Try and make get_system_path work on Linux

Signed-off-by: paul.profizi <[email protected]>

* Fix style in test

Signed-off-by: paul.profizi <[email protected]>

* Confirmed the old hack is no longer required

Signed-off-by: paul.profizi <[email protected]>

---------

Signed-off-by: paul.profizi <[email protected]>

Author: PProfizi

src/ansys/dpf/core/server_factory.py

@@ -602,13 +602,6 @@ def get_server_type_from_config(
         if config.protocol == CommunicationProtocols.gRPC and config.legacy:
             return LegacyGrpcServer
         elif config.protocol == CommunicationProtocols.gRPC and not config.legacy:
-            if ansys_path is None:
-                from ansys.dpf.core.misc import __ansys_version__
-
-                ansys_path = os.environ.get("AWP_ROOT" + str(__ansys_version__), None)
-            if ansys_path is not None:
-                sub_folders = os.path.join(ansys_path, _get_path_in_install())
-                os.environ["PATH"] += sub_folders
             return GrpcServer
         elif config.protocol == CommunicationProtocols.InProcess and not config.legacy:
             return InProcessServer

src/ansys/dpf/core/server_types.py

@@ -9,6 +9,7 @@
 import os
 import socket
 import subprocess
+import sys
 import time
 import warnings
 import traceback
@@ -927,14 +928,7 @@ def __init__(
             # Forced to use ctypes to get the updated PATH due to sys.exec not the Python
             # interpreter when running Python plugin test VS project
             # The better solution would be to not need to update the path
-            windll.kernel32.GetEnvironmentVariableA.argtypes = (c_char_p, c_char_p, c_int)
-            windll.kernel32.GetEnvironmentVariableA.restype = c_int
-            name = "PATH"
-            b_name = name.encode("utf-8")
-            size = 32767
-            buffer = create_string_buffer(b"", size)
-            _ = windll.kernel32.GetEnvironmentVariableA(b_name, buffer, size)
-            os.environ["PATH"] = buffer.value.decode("utf-8")
+            os.environ["PATH"] = get_system_path()
 
     @property
     def version(self):
@@ -977,6 +971,22 @@ def config(self):
         return server_factory.AvailableServerConfigs.InProcessServer
 
 
+def get_system_path() -> str:
+    """Return the current PATH environment variable value of the system."""
+    if not os.name == "posix":
+        windll.kernel32.GetEnvironmentVariableA.argtypes = (c_char_p, c_char_p, c_int)
+        windll.kernel32.GetEnvironmentVariableA.restype = c_int
+        name = "PATH"
+        b_name = name.encode("utf-8")
+        size = 32767
+        buffer = create_string_buffer(b"", size)
+        _ = windll.kernel32.GetEnvironmentVariableA(b_name, buffer, size)
+        return buffer.value.decode("utf-8")
+    else:
+        return sys.path
+
+
+
 class LegacyGrpcServer(BaseServer):
     """Provides an instance of the DPF server using InProcess gRPC.
     Kept for backward-compatibility with dpf servers <0.5.0.

tests/test_server.py

@@ -91,6 +91,13 @@ def test_start_local_server(self, server_config):
         assert has_local_server()
         shutdown_all_session_servers()
 
+    def test_server_env_path_cleanup(self, server_config):
+        """Test that running and stopping servers does not pollute the system PATH."""
+        from ansys.dpf.core.server_types import get_system_path
+        path_len_init = len(get_system_path())
+        _ = dpf.core.start_local_server(config=server_config)
+        assert len(get_system_path()) == path_len_init
+
     def test_start_local_server_with_config(self, server_config):
         set_server_configuration(None)
         shutdown_all_session_servers()