Merge pull request #274 from lsst/tickets/DM-41832

DM-41832: Use spawn as start method, deprecate fork option

.github/workflows/build.yaml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ["3.10", "3.11"]
+        python-version: ["3.11"]
 
     steps:
       - uses: actions/checkout@v3
@@ -79,7 +79,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: "3.10"
+          python-version: "3.11"
 
       - name: Install dependencies
         run: |

.github/workflows/build_docs.yaml

@@ -18,7 +18,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v3
         with:
-          python-version: '3.10'
+          python-version: '3.11'
           cache: "pip"
           cache-dependency-path: "setup.cfg"
 
@@ -35,7 +35,7 @@ jobs:
         run: pip install --no-deps -v .
 
       - name: Install documenteer
-        run: pip install 'documenteer[pipelines]<0.8'
+        run: pip install 'documenteer[pipelines]>=0.8'
 
       - name: Build documentation
         working-directory: ./doc

.pre-commit-config.yaml

@@ -1,13 +1,13 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
       - id: check-yaml
       - id: end-of-file-fixer
       - id: trailing-whitespace
       - id: check-toml
   - repo: https://github.com/psf/black-pre-commit-mirror
-    rev: 23.9.1
+    rev: 23.11.0
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python
@@ -22,6 +22,6 @@ repos:
         name: isort (python)
   - repo: https://github.com/astral-sh/ruff-pre-commit
     # Ruff version.
-    rev: v0.0.289
+    rev: v0.1.6
     hooks:
       - id: ruff

doc/changes/DM-41832.removal.md

@@ -0,0 +1,3 @@
+Support for fork option in `pipetask run` has been removed as unsafe.
+Default start option now is `spawn`, `forkserver` is also available.
+The `fork` option is still present in CLI for compatibility, but is deprecated and replaced by `spawn` if specified.

python/lsst/ctrl/mpexec/cli/opt/options.py

@@ -429,7 +429,10 @@
     "--start-method",
     default=None,
     type=click.Choice(choices=["spawn", "fork", "forkserver"]),
-    help="Multiprocessing start method, default is platform-specific.",
+    help=(
+        "Multiprocessing start method, default is platform-specific. "
+        "Fork method is no longer supported, spawn is used instead if fork is selected."
+    ),
 )
 
 

python/lsst/ctrl/mpexec/cli/script/run.py

@@ -182,6 +182,11 @@ def run(  # type: ignore
         function and pass all the option kwargs to each of the script functions
         which ignore these unused kwargs.
     """
+    # Fork option still exists for compatibility but we use spawn instead.
+    if start_method == "fork":
+        start_method = "spawn"
+        _log.warning("Option --start-method=fork is unsafe and no longer supported, will use spawn instead.")
+
     args = SimpleNamespace(
         pdb=pdb,
         graph_fixup=graph_fixup,

python/lsst/ctrl/mpexec/cli/script/run_qbb.py

@@ -25,10 +25,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import logging
 from types import SimpleNamespace
 
 from ... import CmdLineFwk, TaskFactory
 
+_log = logging.getLogger(__name__)
+
 
 def run_qbb(
     butler_config: str,
@@ -97,6 +100,11 @@ def run_qbb(
         implies no limit. The string can be either a single integer (implying
         units of MB) or a combination of number and unit.
     """
+    # Fork option still exists for compatibility but we use spawn instead.
+    if start_method == "fork":
+        start_method = "spawn"
+        _log.warning("Option --start-method=fork is unsafe and no longer supported, will use spawn instead.")
+
     args = SimpleNamespace(
         butler_config=butler_config,
         qgraph=qgraph,

python/lsst/ctrl/mpexec/mpGraphExecutor.py

@@ -100,7 +100,7 @@ def terminated(self) -> bool:
     def start(
         self,
         quantumExecutor: QuantumExecutor,
-        startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None,
+        startMethod: Literal["spawn"] | Literal["forkserver"],
     ) -> None:
         """Start process which runs the task.
 
@@ -119,11 +119,13 @@ def start(
         logConfigState = CliLog.configState
 
         mp_ctx = multiprocessing.get_context(startMethod)
-        self.process = mp_ctx.Process(
+        self.process = mp_ctx.Process(  # type: ignore[attr-defined]
             target=_Job._executeJob,
             args=(quantumExecutor, taskDef, quantum_pickle, logConfigState, snd_conn),
             name=f"task-{self.qnode.quantum.dataId}",
         )
+        # mypy is getting confused by multiprocessing.
+        assert self.process is not None
         self.process.start()
         self.started = time.time()
         self._state = JobState.RUNNING
@@ -268,7 +270,7 @@ def submit(
         self,
         job: _Job,
         quantumExecutor: QuantumExecutor,
-        startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None,
+        startMethod: Literal["spawn"] | Literal["forkserver"],
     ) -> None:
         """Submit one more job for execution
 
@@ -380,7 +382,7 @@ def __init__(
         timeout: float,
         quantumExecutor: QuantumExecutor,
         *,
-        startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None,
+        startMethod: Literal["spawn"] | Literal["forkserver"] | None = None,
         failFast: bool = False,
         pdb: str | None = None,
         executionGraphFixup: ExecutionGraphFixup | None = None,
@@ -393,11 +395,9 @@ def __init__(
         self.executionGraphFixup = executionGraphFixup
         self.report: Report | None = None
 
-        # We set default start method as spawn for MacOS and fork for Linux;
-        # None for all other platforms to use multiprocessing default.
+        # We set default start method as spawn for all platforms.
         if startMethod is None:
-            methods = dict(linux="fork", darwin="spawn")
-            startMethod = methods.get(sys.platform)  # type: ignore
+            startMethod = "spawn"
         self.startMethod = startMethod
 
     def execute(self, graph: QuantumGraph) -> None: