Increases the retry attempts to check for matlab-proxy-app readiness during startup.

fixes mathworks/matlab-proxy#110

Author: krisctl

matlab_proxy_manager/lib/api.py

@@ -1,4 +1,4 @@
-# Copyright 2024 The MathWorks, Inc.
+# Copyright 2024-2025 The MathWorks, Inc.
 import asyncio
 import os
 import secrets
@@ -147,27 +147,28 @@ async def _start_subprocess_and_check_for_readiness(
         return None
 
     process_id, url, mwi_base_url = result
-    server_process = None
 
-    # Check for the matlab proxy server readiness
-    if helpers.is_server_ready(url=f"{url}{mwi_base_url}", backoff_factor=0.5):
-        log.debug("Matlab proxy process info: %s, %s", url, mwi_base_url)
-        server_process = ServerProcess(
-            server_url=url,
-            mwi_base_url=mwi_base_url,
-            headers=helpers.convert_mwi_env_vars_to_header_format(
-                matlab_proxy_env, "MWI"
-            ),
-            pid=str(process_id),
-            parent_pid=ctx,
-            id=key,
-            type="shared" if is_shared_matlab else "named",
-            mpm_auth_token=mpm_auth_token,
-        )
-    else:
-        log.error("matlab-proxy server never became ready")
+    log.debug("Matlab proxy process info: %s, %s", url, mwi_base_url)
+    matlab_proxy_process = ServerProcess(
+        server_url=url,
+        mwi_base_url=mwi_base_url,
+        headers=helpers.convert_mwi_env_vars_to_header_format(matlab_proxy_env, "MWI"),
+        pid=str(process_id),
+        parent_pid=ctx,
+        id=key,
+        type="shared" if is_shared_matlab else "named",
+        mpm_auth_token=mpm_auth_token,
+    )
 
-    return server_process
+    # Check for the matlab proxy server readiness
+    if not helpers.is_server_ready(
+        url=matlab_proxy_process.absolute_url, retries=7, backoff_factor=0.5
+    ):
+        log.error("MATLAB Proxy Server unavailable: matlab-proxy-app failed to start or has timed out.")
+        matlab_proxy_process.shutdown()
+        matlab_proxy_process = None
+
+    return matlab_proxy_process
 
 
 def _prepare_cmd_and_env_for_matlab_proxy():

matlab_proxy_manager/storage/server.py

@@ -1,9 +1,11 @@
-# Copyright 2024 The MathWorks, Inc.
+# Copyright 2024-2025 The MathWorks, Inc.
 import json
 from dataclasses import asdict, dataclass, field
 from pathlib import Path
 from typing import Optional
 
+import psutil
+
 from matlab_proxy_manager.utils import helpers, logger
 
 log = logger.get()
@@ -96,8 +98,31 @@ def shutdown(self):
             return shutdown_resp
         except Exception as e:
             log.debug("Exception while shutting down matlab proxy: %s", e)
+
+            # Force kill matlab-proxy and its process tree if termination
+            # via shutdown_integration endpoint fails
+            matlab_proxy_process = psutil.Process(int(self.pid))
+            self.terminate_process_tree(matlab_proxy_process)
             return None
 
+    def terminate_process_tree(self, matlab_proxy_process):
+        """
+        Terminates the process tree of the MATLAB proxy server.
+
+        This method terminates all child processes of the MATLAB proxy process,
+        waits for a short period, and then forcefully kills any remaining processes.
+        Finally, it kills the main MATLAB proxy process itself.
+        """
+        matlab_proxy_child_processes = matlab_proxy_process.children(recursive=True)
+        for child_process in matlab_proxy_child_processes:
+            child_process.terminate()
+        _, alive = psutil.wait_procs(matlab_proxy_child_processes, timeout=5)
+        for process in alive:
+            # Kill the processes which are alive even after waiting for 5 seconds
+            process.kill()
+        matlab_proxy_process.kill()
+        log.debug("Killed matlab-proxy process with PID: %s", self.pid)
+
     def is_server_alive(self) -> bool:
         """
         Checks if the server process is alive and ready.

tests/unit/proxy-manager/lib/test_api.py

@@ -1,4 +1,4 @@
-# Copyright 2024 The MathWorks, Inc.
+# Copyright 2024-2025 The MathWorks, Inc.
 import pytest
 
 from matlab_proxy_manager.lib import api as mpm_api
@@ -177,6 +177,66 @@ async def test_start_matlab_proxy_returns_none_if_server_not_created(
     assert result is None
 
 
+async def test_matlab_proxy_is_cleaned_up_if_server_was_not_ready(mocker):
+    """
+    Test case for cleaning up MATLAB proxy when server is not ready.
+
+    This test mocks various dependencies and verifies the behavior of the
+    _start_matlab_proxy function when no existing server is found and
+    a new server is created but not ready. It checks if the function correctly
+    returns None, calls the expected methods, and cleans up the server.
+    """
+    mock_find_existing_server = mocker.patch(
+        "matlab_proxy_manager.storage.server.ServerProcess.find_existing_server",
+        return_value=None,
+    )
+    mock_shutdown = mocker.patch(
+        "matlab_proxy_manager.storage.server.ServerProcess.shutdown",
+        return_value=None,
+    )
+    mock_delete_dangling_servers = mocker.patch(
+        "matlab_proxy_manager.utils.helpers._are_orphaned_servers_deleted",
+        return_value=None,
+    )
+    mock_create_state_file = mocker.patch(
+        "matlab_proxy_manager.utils.helpers.create_state_file", return_value=None
+    )
+    mock_create_proxy_manager_dir = mocker.patch(
+        "matlab_proxy_manager.utils.helpers.create_and_get_proxy_manager_data_dir",
+        return_value=None,
+    )
+    mock_is_server_ready = mocker.patch(
+        "matlab_proxy_manager.utils.helpers.is_server_ready", return_value=False
+    )
+    mock_prep_cmd_and_env = mocker.patch(
+        "matlab_proxy_manager.lib.api._prepare_cmd_and_env_for_matlab_proxy",
+        return_value=([], {}),
+    )
+    mock_start_subprocess = mocker.patch(
+        "matlab_proxy_manager.lib.api._start_subprocess",
+        return_value=(1, "dummy", "dummy"),
+    )
+
+    caller_id = "test_caller"
+    parent_id = "test_parent"
+    is_shared_matlab = True
+
+    result = await mpm_api._start_matlab_proxy(
+        caller_id=caller_id, ctx=parent_id, is_shared_matlab=is_shared_matlab
+    )
+
+    mock_delete_dangling_servers.assert_called_once_with(parent_id)
+    mock_create_proxy_manager_dir.assert_called_once()
+    mock_find_existing_server.assert_called_once()
+    mock_create_state_file.assert_not_called()
+    mock_prep_cmd_and_env.assert_called_once()
+    mock_start_subprocess.assert_awaited_once()
+    mock_is_server_ready.assert_called_once()
+    mock_shutdown.assert_called_once()
+
+    assert result is None
+
+
 # Test for shutdown with missing arguments
 async def test_shutdown_missing_args(mocker, mock_server_process):
     """

tests/unit/proxy-manager/storage/test_server.py

@@ -1,4 +1,4 @@
-# Copyright 2024 The MathWorks, Inc.
+# Copyright 2024-2025 The MathWorks, Inc.
 from pathlib import Path
 
 import pytest
@@ -52,6 +52,9 @@ def test_shutdown_exception(mocker, server_process):
     mock_req_retry_session.return_value.delete.side_effect = Exception(
         "Server unreachable"
     )
+    mock_psutil_process = mocker.patch(
+        "psutil.Process", return_value=mocker.MagicMock()
+    )
 
     shutdown_response = server_process.shutdown()
 
@@ -61,6 +64,7 @@ def test_shutdown_exception(mocker, server_process):
         headers={"Dummy_header": "Dummy_value"},
     )
     assert shutdown_response is None
+    mock_psutil_process.return_value.kill.assert_called()
 
 
 def test_server_process_instantiation_from_string_correctly():