diff --git a/crawl4ai/browser_manager.py b/crawl4ai/browser_manager.py index f4ab0aa32..4d0028a10 100644 --- a/crawl4ai/browser_manager.py +++ b/crawl4ai/browser_manager.py @@ -244,20 +244,27 @@ async def start(self) -> str: # Start browser process try: # Use DETACHED_PROCESS flag on Windows to fully detach the process - # On Unix, we'll use preexec_fn=os.setpgrp to start the process in a new process group + # On Unix, use start_new_session=True to start the process in a new + # session/process group. preexec_fn=os.setpgrp runs Python code + # between fork() and exec() in the child, which is unsafe in a + # multi-threaded parent (e.g. a gunicorn/uvicorn worker) since the + # child only inherits the calling thread and can deadlock or crash + # on a lock (allocator, import lock) held by another thread at + # fork time. start_new_session=True performs the equivalent + # setsid() natively without running Python in the child. if sys.platform == "win32": self.browser_process = subprocess.Popen( - args, - stdout=subprocess.PIPE, + args, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, creationflags=subprocess.DETACHED_PROCESS | subprocess.CREATE_NEW_PROCESS_GROUP ) else: self.browser_process = subprocess.Popen( - args, - stdout=subprocess.PIPE, + args, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, - preexec_fn=os.setpgrp # Start in a new process group + start_new_session=True # Start in a new session/process group ) # If verbose is True print args used to run the process diff --git a/tests/browser/test_managed_browser_start_new_session.py b/tests/browser/test_managed_browser_start_new_session.py new file mode 100644 index 000000000..531796661 --- /dev/null +++ b/tests/browser/test_managed_browser_start_new_session.py @@ -0,0 +1,45 @@ +"""Regression test for issue #2238. + +ManagedBrowser.start() used preexec_fn=os.setpgrp to put the launched +Chromium in its own process group. preexec_fn runs arbitrary Python code +between fork() and exec() in the child process; in a multi-threaded parent +(e.g. a gunicorn/uvicorn worker) the child only inherits the calling +thread, so a lock held by another thread at fork time (allocator, import +lock, ...) can leave the child deadlocked or crashing before exec ever +runs. start_new_session=True achieves the same "own process group" result +via setsid() without executing Python in the forked child. +""" +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from crawl4ai.browser_manager import ManagedBrowser +from crawl4ai.async_configs import BrowserConfig + + +@pytest.mark.asyncio +async def test_start_uses_start_new_session_not_preexec_fn(): + browser_config = BrowserConfig( + browser_type="chromium", + headless=True, + debugging_port=9222, + host="localhost", + ) + manager = ManagedBrowser(browser_config=browser_config, logger=MagicMock()) + manager._get_browser_args = AsyncMock(return_value=["/fake/chrome"]) + manager._initial_startup_check = AsyncMock(return_value=None) + + fake_process = MagicMock() + fake_process.poll.return_value = None + + with patch("crawl4ai.browser_manager.sys.platform", "linux"), \ + patch("crawl4ai.browser_manager.subprocess.Popen", return_value=fake_process) as mock_popen, \ + patch("crawl4ai.browser_manager.subprocess.check_output", side_effect=FileNotFoundError), \ + patch("asyncio.sleep", new=AsyncMock(return_value=None)): + await manager.start() + + assert mock_popen.called + _, kwargs = mock_popen.call_args + assert kwargs.get("start_new_session") is True + assert "preexec_fn" not in kwargs