matlab-proxy derives port information for communication from spawned MATLAB process. Users will now see the MATLAB UI only after it has initialized fully.

Author: krisctl

gui/src/components/Controls/index.js

@@ -102,10 +102,10 @@ MATLAB version: ${matlabVersion}%0D%0A`,
                 onClick={() => callback(Confirmations.START)}
                 disabled={!licensed || matlabStarting || matlabStopping || (authEnabled && !isAuthenticated)}
                 data-for="control-button-tooltip"
-                data-tip={`${matlabRunning ? 'Restart' : 'Start'}  your MATLAB session`}
+                data-tip={`${matlabRunning ? 'Restart' : 'Start'}  MATLAB`}
             >
                 <span className={`icon-custom-${matlabRunning ? 're' : ''}start`}></span>
-                <span className='btn-label'>{`${matlabRunning ? 'Restart' : 'Start'} MATLAB Session`}</span>
+                <span className='btn-label'>{`${matlabRunning ? 'Restart' : 'Start'} MATLAB`}</span>
             </button>
             <button
                 id="stopMatlab"
@@ -114,10 +114,10 @@ MATLAB version: ${matlabVersion}%0D%0A`,
                 onClick={() => callback(Confirmations.STOP)}
                 disabled={!matlabRunning || (authEnabled && !isAuthenticated)}
                 data-for="control-button-tooltip"
-                data-tip="Stop your MATLAB session"
+                data-tip="Stop MATLAB"
             >
                 <span className='icon-custom-stop'></span>
-                <span className='btn-label'>Stop MATLAB Session</span>
+                <span className='btn-label'>Stop MATLAB</span>
             </button>
             <button
                 id="unsetLicensing"

matlab_proxy/app_state.py

@@ -1,18 +1,13 @@
 # Copyright (c) 2020-2022 The MathWorks, Inc.
 
 import asyncio
-import errno
 import json
 import logging
 import os
-import socket
-import sys
 import time
 from collections import deque
 from datetime import datetime, timedelta, timezone
 
-import aiohttp
-
 from matlab_proxy import util
 from matlab_proxy.util import mw, mwi, system, windows
 from matlab_proxy.util.mwi import environment_variables as mwi_env
@@ -28,6 +23,7 @@
     XvfbError,
     log_error,
 )
+from matlab_proxy.constants import CONNECTOR_SECUREPORT_FILENAME
 
 logger = mwi.logger.get()
 
@@ -37,6 +33,10 @@ class AppState:
     This class handles state of MATLAB, MATLAB Licensing and Xvfb.
     """
 
+    # Constants that are applicable to AppState class
+    MATLAB_PORT_CHECK_DELAY_IN_SECONDS = 1
+    EMBEDDED_CONNECTOR_MAX_STARTUP_DURATION_IN_SECONDS = 120
+
     def __init__(self, settings):
         """Parameterized constructor for the AppState class.
         Initializes member variables and checks for an existing MATLAB installation.
@@ -55,9 +55,6 @@ def __init__(self, settings):
 
         # Dictionary of all files used to manage the MATLAB session.
         self.matlab_session_files = {
-            # The file created by this instance of matlab-proxy to signal to other matlab-proxy processes
-            # that this self.matlab_port will be used by this instance.
-            "mwi_proxy_lock_file": None,
             # The file created and written by MATLAB's Embedded connector to signal readiness.
             "matlab_ready_file": None,
         }
@@ -210,6 +207,9 @@ async def get_matlab_state(self):
             if matlab is None or not matlab.is_running():
                 return "down"
 
+        if not self.matlab_session_files["matlab_ready_file"].exists():
+            return "starting"
+
         # If execution reaches this else block, it implies that:
         # 1) MATLAB process has started.
         # 2) Embedded connector has not started yet.
@@ -395,14 +395,8 @@ def persist_licensing(self):
             with open(cached_licensing_file, "w") as f:
                 f.write(json.dumps(self.licensing))
 
-    def prepare_lock_files_for_MATLAB_launch(self):
-        """Finds and reserves a free port for MATLAB Embedded Connector in the allowed range.
-        Creates the lock file to prevent any other matlab-proxy process to use the reserved port of this
-        process.
-
-        Raises:
-            e: socket.error if the exception raised is other than port already occupied.
-        """
+    def create_logs_dir_for_MATLAB(self):
+        """Creates the root folder where MATLAB writes the ready file and updates attibutes on self."""
 
         # NOTE It is not guranteed that the port will remain free!
         # FIXME Because of https://github.com/http-party/node-http-proxy/issues/1342 the
@@ -414,77 +408,23 @@ def prepare_lock_files_for_MATLAB_launch(self):
         ):
             return 31515
         else:
-            # TODO If MATLAB Connector is enhanced to allow any port, then the
-            # following can be used to get an unused port instead of the for loop and
-            # try-except.
-            # s.bind(("", 0))
-            # self.matlab_port = s.getsockname()[1]
+            mwi_logs_root_dir = self.settings["mwi_logs_root_dir"]
+            # Use the app_port number to identify the server as that is user visible
+            mwi_logs_dir = mwi_logs_root_dir / str(self.settings["app_port"])
 
-            for port in mw.range_matlab_connector_ports():
-                try:
-                    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-                    s.bind(("", port))
-
-                    mwi_logs_root_dir = self.settings["mwi_logs_root_dir"]
-
-                    # The mwi_proxy.lock file indicates to any other matlab-proxy processes
-                    # that this self.matlab_port number is taken up by this process.
-                    mwi_proxy_lock_file = mwi_logs_root_dir / (
-                        self.settings["mwi_proxy_lock_file_name"] + "." + str(port)
-                    )
+            # Create a folder to hold the matlab_ready_file that will be created by MATLAB to signal readiness
+            # This is the same folder to which MATLAB will write logs to.
+            mwi_logs_dir.mkdir(parents=True, exist_ok=True)
 
-                    # Check if the mwi_proxy_lock_file exists.
-                    # Implies there was a competing matlab-proxy process which found the same port before this process
-                    if mwi_proxy_lock_file.exists():
-                        logger.debug(
-                            f"Skipping port number {port} for MATLAB as lock file already exists at {mwi_proxy_lock_file}"
-                        )
-                        s.close()
-
-                    else:
-                        # Use the app_port number to identify the server as that is user visible
-                        mwi_logs_dir = mwi_logs_root_dir / str(
-                            self.settings["app_port"]
-                        )
-
-                        # Create a folder to hold the matlab_ready_file that will be created by MATLAB to signal readiness.
-                        # This is the same folder to which MATLAB will write logs to.
-                        mwi_logs_dir.mkdir(parents=True, exist_ok=True)
-
-                        # Create the lock file first to minimize the critical section.
-                        mwi_proxy_lock_file.touch()
-                        logger.info(
-                            f"Communicating with MATLAB on port:{port}, lock file: {mwi_proxy_lock_file}"
-                        )
-
-                        # Created by MATLAB when it is ready to service requests.
-                        matlab_ready_file = mwi_logs_dir / "connector.securePort"
-
-                        # Update member variables of AppState class
-                        # Store the port number on which MATLAB will be launched for this matlab-proxy process.
-                        self.matlab_port = port
-                        self.mwi_logs_dir = mwi_logs_dir
-                        self.matlab_session_files[
-                            "mwi_proxy_lock_file"
-                        ] = mwi_proxy_lock_file
-                        self.matlab_session_files[
-                            "matlab_ready_file"
-                        ] = matlab_ready_file
-                        s.close()
-
-                        logger.debug(
-                            f"matlab_session_files:{self.matlab_session_files}"
-                        )
-                        return
+            # Created by MATLAB when it is ready to service requests
+            matlab_ready_file = mwi_logs_dir / CONNECTOR_SECUREPORT_FILENAME
 
-                # For windows container's (when testing in github workflows) PermissionError and in linux, OSError is
-                # thrown when trying to bind a used port from a previous test instead of the expected socket.error
-                except (OSError, PermissionError) as e:
-                    pass
+            # Update member variables of AppState class
+            self.mwi_logs_dir = mwi_logs_dir
+            self.matlab_session_files["matlab_ready_file"] = matlab_ready_file
 
-                except socket.error as e:
-                    if e.errno != errno.EADDRINUSE:
-                        raise e
+            logger.debug(f"matlab_session_files:{self.matlab_session_files}")
+            return
 
     def create_server_info_file(self):
         mwi_logs_root_dir = self.settings["mwi_logs_root_dir"]
@@ -582,10 +522,6 @@ async def __setup_env_for_matlab(self) -> dict:
             # Adding DISPLAY key which is only available after starting Xvfb successfully.
             matlab_env["DISPLAY"] = self.settings["matlab_display"]
 
-        # MW_CONNECTOR_SECURE_PORT and MATLAB_LOG_DIR keys to matlab_env as they are available after
-        # reserving port and preparing lockfiles for MATLAB
-        matlab_env["MW_CONNECTOR_SECURE_PORT"] = str(self.matlab_port)
-
         # The matlab ready file is written into this location(self.mwi_logs_dir) by MATLAB
         # The mwi_logs_dir is where MATLAB will write any subsequent logs
         matlab_env["MATLAB_LOG_DIR"] = str(self.mwi_logs_dir)
@@ -726,28 +662,28 @@ async def start_matlab(self, restart_matlab=False):
             self.processes["xvfb"] = xvfb
 
         try:
-            # Finds and reserves a free port, then prepare lock files for the MATLAB process.
-            self.prepare_lock_files_for_MATLAB_launch()
+            # Prepare ready file for the MATLAB process.
+            self.create_logs_dir_for_MATLAB()
 
             # Configure the environment MATLAB needs to start
             matlab_env = await self.__setup_env_for_matlab()
 
             logger.debug(
-                "Prepared lock files and configured the environment for MATLAB startup"
+                "Prepared ready file and configured the environment for MATLAB startup"
             )
 
-        # If there's something wrong with setting up lock files or env setup for starting matlab, capture the error for logging
+        # If there's something wrong with setting up files or env setup for starting matlab, capture the error for logging
         # and to pass to the front-end. Halt MATLAB process startup by returning early
         except Exception as err:
             self.error = err
             log_error(logger, err)
             # stop_matlab() does the teardown work by removing any residual files and processes created till now
-            # which is Xvfb process creation and preparing lock files for the MATLAB process.
+            # which is Xvfb process creation and ready file for the MATLAB process.
             await self.stop_matlab()
             return
 
         # Start MATLAB Process
-        logger.debug(f"Starting MATLAB on port {self.matlab_port}")
+        logger.debug("Starting MATLAB")
 
         matlab = await self.__start_matlab_process(matlab_env)
 
@@ -830,9 +766,45 @@ async def matlab_stderr_reader():
                         )
                         await asyncio.sleep(1)
 
-        loop = util.get_event_loop()
+        async def update_matlab_port(delay: int):
+            """Task to populate matlab_port from the matlab ready file. Times out if max_duration is breached
+
+            Args:
+                delay (int): time delay in seconds before retrying the file read operation
+            """
+            logger.debug(
+                f'updating matlab_port information from {self.matlab_session_files["matlab_ready_file"]}'
+            )
+            try:
+                await asyncio.wait_for(
+                    __read_matlab_ready_file(delay),
+                    self.EMBEDDED_CONNECTOR_MAX_STARTUP_DURATION_IN_SECONDS,
+                )
+            except asyncio.TimeoutError:
+                logger.debug(
+                    "Timeout error received while updating matlab port, stopping matlab!"
+                )
+                await self.stop_matlab(force_quit=True)
+                self.error = MatlabError(
+                    "Unable to start MATLAB because of a timeout. Try again by clicking Start MATLAB."
+                )
+
+        async def __read_matlab_ready_file(delay):
+            # reads with delays from the file where connector has written its port information
+            while not self.matlab_session_files["matlab_ready_file"].exists():
+                await asyncio.sleep(delay)
 
+            with open(self.matlab_session_files["matlab_ready_file"]) as f:
+                self.matlab_port = int(f.read())
+                logger.debug(
+                    f"MATLAB Ready file successfully read, matlab_port set to: {self.matlab_port}"
+                )
+
+        loop = util.get_event_loop()
         self.tasks["matlab_stderr_reader"] = loop.create_task(matlab_stderr_reader())
+        self.tasks["update_matlab_port"] = loop.create_task(
+            update_matlab_port(self.MATLAB_PORT_CHECK_DELAY_IN_SECONDS)
+        )
 
     """
     async def __send_terminate_integration_request(self):
@@ -1014,17 +986,18 @@ async def stop_matlab(self, force_quit=False):
             for waiter in waiters:
                 await waiter
 
-        stderr_reader_task = self.tasks.get("matlab_stderr_reader")
-        if stderr_reader_task is not None:
-            try:
-                stderr_reader_task.cancel()
-                await stderr_reader_task
-            except asyncio.CancelledError:
-                pass
-
-            self.tasks.pop("matlab_stderr_reader")
+        # Canceling all the async tasks in the list
+        for name, task in list(self.tasks.items()):
+            if task:
+                try:
+                    task.cancel()
+                    await task
+                except asyncio.CancelledError:
+                    pass
+            logger.debug(f"{name} task stopped successfully")
 
-            logger.info("matlab_stderr_reader() task: Stopped successfully.")
+        # After stopping all the tasks, set self.tasks to empty dict
+        self.tasks = {}
 
         # Clear logs if MATLAB stopped intentionally
         logger.debug("Clearing logs!")

matlab_proxy/constants.py

@@ -0,0 +1,3 @@
+"""This module defines project-level constants"""
+
+CONNECTOR_SECUREPORT_FILENAME = "connector.securePort"

matlab_proxy/devel.py

@@ -11,6 +11,8 @@
 
 from matlab_proxy import settings
 from matlab_proxy.util.mwi import environment_variables as mwi_env
+from matlab_proxy.constants import CONNECTOR_SECUREPORT_FILENAME
+from matlab_proxy.util.event_loop import *
 
 desktop_html = b"""
 <h1>Fake MATLAB Web Desktop</h1>
@@ -27,23 +29,13 @@
 """
 
 
-def wait_for_port(port):
-    """Waits for the given port to become available
-
-    Args:
-        port (Integer): Port number to start fake matlab server.
-    """
-    while True:
-        print(f"Waiting for port {port} to be available")
-        try:
-            s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-            s.bind(("", port))
-        except OSError:
-            time.sleep(5)
-            continue
-        # Once successful, close the port and stop waiting
-        s.close()
-        break
+def assign_free_port():
+    """Finds an available free port"""
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    s.bind(("", 0))
+    port = s.getsockname()[1]
+    s.close()
+    return port
 
 
 async def web_handler(request):
@@ -176,10 +168,11 @@ async def fake_matlab_started(app):
 
     # Real MATLAB always uses  $MATLAB_LOG_DIR/connection.securePort as the ready file
     # We mock reading from the environment variable by calling the helper functions
-    mwi_logs_dir = settings.get(dev=True)["mwi_logs_root_dir"] / str(app["port"])
-    mwi_logs_dir.mkdir(parents=True, exist_ok=True)
+    matlab_logs_dir = os.getenv(mwi_env.get_env_name_matlab_log_dir())
 
-    app["matlab_ready_file"] = mwi_logs_dir / "connector.securePort"
+    app["matlab_ready_file"] = Path(
+        f"{matlab_logs_dir}/{CONNECTOR_SECUREPORT_FILENAME}"
+    )
 
     ready_delay = app["ready_delay"]
     try:
@@ -188,6 +181,10 @@ async def fake_matlab_started(app):
             f"Creating fake MATLAB Embedded Connector ready file at {app['matlab_ready_file']}"
         )
         app["matlab_ready_file"].touch()
+
+        # Populate ready file with the embedded connector port information
+        with open(app["matlab_ready_file"], "w") as f:
+            f.write(str(app["port"]))
     except asyncio.CancelledError:
         pass
 
@@ -223,8 +220,7 @@ def matlab(args):
     Args:
         args (Dict): Contains data on how to start web server.
     """
-    port = int(os.environ["MW_CONNECTOR_SECURE_PORT"])
-    wait_for_port(port)
+    port = assign_free_port()
     print(f"Serving fake MATLAB Embedded Connector at port {port}")
     app = web.Application()
     app["ready_delay"] = args.ready_delay