Skip to content

Commit 3b7968f

Browse files
authored
fix: enhance process close procedure (#1213)
1 parent 0a7f8a3 commit 3b7968f

File tree

5 files changed

+211
-17
lines changed

5 files changed

+211
-17
lines changed

playwright/_impl/_connection.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ def __init__(
180180
self.playwright_future: asyncio.Future["Playwright"] = loop.create_future()
181181
self._error: Optional[BaseException] = None
182182
self.is_remote = False
183+
self._init_task: Optional[asyncio.Task] = None
183184

184185
def mark_as_remote(self) -> None:
185186
self.is_remote = True
@@ -196,12 +197,13 @@ async def init() -> None:
196197
self.playwright_future.set_result(await self._root_object.initialize())
197198

198199
await self._transport.connect()
199-
self._loop.create_task(init())
200+
self._init_task = self._loop.create_task(init())
200201
await self._transport.run()
201202

202203
def stop_sync(self) -> None:
203204
self._transport.request_stop()
204205
self._dispatcher_fiber.switch()
206+
self._loop.run_until_complete(self._transport.wait_until_stopped())
205207
self.cleanup()
206208

207209
async def stop_async(self) -> None:
@@ -210,6 +212,8 @@ async def stop_async(self) -> None:
210212
self.cleanup()
211213

212214
def cleanup(self) -> None:
215+
if self._init_task and not self._init_task.done():
216+
self._init_task.cancel()
213217
for ws_connection in self._child_ws_connections:
214218
ws_connection._transport.dispose()
215219
self.emit("close")

playwright/_impl/_transport.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ def request_stop(self) -> None:
110110

111111
async def wait_until_stopped(self) -> None:
112112
await self._stopped_future
113-
await self._proc.wait()
114113

115114
async def connect(self) -> None:
116115
self._stopped_future: asyncio.Future = asyncio.Future()
@@ -147,22 +146,30 @@ async def run(self) -> None:
147146
while not self._stopped:
148147
try:
149148
buffer = await self._proc.stdout.readexactly(4)
149+
if self._stopped:
150+
break
150151
length = int.from_bytes(buffer, byteorder="little", signed=False)
151152
buffer = bytes(0)
152153
while length:
153154
to_read = min(length, 32768)
154155
data = await self._proc.stdout.readexactly(to_read)
156+
if self._stopped:
157+
break
155158
length -= to_read
156159
if len(buffer):
157160
buffer = buffer + data
158161
else:
159162
buffer = data
163+
if self._stopped:
164+
break
160165

161166
obj = self.deserialize_message(buffer)
162167
self.on_message(obj)
163168
except asyncio.IncompleteReadError:
164169
break
165170
await asyncio.sleep(0)
171+
172+
await self._proc.wait()
166173
self._stopped_future.set_result(None)
167174

168175
def send(self, message: Dict) -> None:

playwright/async_api/_context_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async def __aenter__(self) -> AsyncPlaywright:
3737
loop.create_task(self._connection.run())
3838
playwright_future = self._connection.playwright_future
3939

40-
done, pending = await asyncio.wait(
40+
done, _ = await asyncio.wait(
4141
{self._connection._transport.on_error_future, playwright_future},
4242
return_when=asyncio.FIRST_COMPLETED,
4343
)

playwright/sync_api/_context_manager.py

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
# limitations under the License.
1414

1515
import asyncio
16-
from typing import Any
16+
import sys
17+
from typing import Any, Optional
1718

1819
from greenlet import greenlet
1920

@@ -29,34 +30,45 @@
2930
class PlaywrightContextManager:
3031
def __init__(self) -> None:
3132
self._playwright: SyncPlaywright
33+
self._loop: asyncio.AbstractEventLoop
34+
self._own_loop = False
35+
self._watcher: Optional[asyncio.AbstractChildWatcher] = None
3236

3337
def __enter__(self) -> SyncPlaywright:
34-
loop: asyncio.AbstractEventLoop
35-
own_loop = None
3638
try:
37-
loop = asyncio.get_running_loop()
39+
self._loop = asyncio.get_running_loop()
3840
except RuntimeError:
39-
loop = asyncio.new_event_loop()
40-
own_loop = loop
41-
if loop.is_running():
41+
self._loop = asyncio.new_event_loop()
42+
self._own_loop = True
43+
if self._loop.is_running():
4244
raise Error(
4345
"""It looks like you are using Playwright Sync API inside the asyncio loop.
4446
Please use the Async API instead."""
4547
)
4648

47-
def greenlet_main() -> None:
48-
loop.run_until_complete(self._connection.run_as_sync())
49+
# In Python 3.7, asyncio.Process.wait() hangs because it does not use ThreadedChildWatcher
50+
# which is used in Python 3.8+. This is unix specific and also takes care about
51+
# cleaning up zombie processes. See https://bugs.python.org/issue35621
52+
if (
53+
sys.version_info[0] == 3
54+
and sys.version_info[1] == 7
55+
and sys.platform != "win32"
56+
and isinstance(asyncio.get_child_watcher(), asyncio.SafeChildWatcher)
57+
):
58+
from ._py37ThreadedChildWatcher import ThreadedChildWatcher # type: ignore
59+
60+
self._watcher = ThreadedChildWatcher()
61+
asyncio.set_child_watcher(self._watcher) # type: ignore
4962

50-
if own_loop:
51-
loop.run_until_complete(loop.shutdown_asyncgens())
52-
loop.close()
63+
def greenlet_main() -> None:
64+
self._loop.run_until_complete(self._connection.run_as_sync())
5365

5466
dispatcher_fiber = greenlet(greenlet_main)
5567
self._connection = Connection(
5668
dispatcher_fiber,
5769
create_remote_object,
58-
PipeTransport(loop, compute_driver_executable()),
59-
loop,
70+
PipeTransport(self._loop, compute_driver_executable()),
71+
self._loop,
6072
)
6173

6274
g_self = greenlet.getcurrent()
@@ -77,3 +89,8 @@ def start(self) -> SyncPlaywright:
7789

7890
def __exit__(self, *args: Any) -> None:
7991
self._connection.stop_sync()
92+
if self._watcher:
93+
self._watcher.close()
94+
if self._own_loop:
95+
self._loop.run_until_complete(self._loop.shutdown_asyncgens())
96+
self._loop.close()
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
2+
# --------------------------------------------
3+
#
4+
# 1. This LICENSE AGREEMENT is between the Python Software Foundation
5+
# ("PSF"), and the Individual or Organization ("Licensee") accessing and
6+
# otherwise using this software ("Python") in source or binary form and
7+
# its associated documentation.
8+
#
9+
# 2. Subject to the terms and conditions of this License Agreement, PSF hereby
10+
# grants Licensee a nonexclusive, royalty-free, world-wide license to reproduce,
11+
# analyze, test, perform and/or display publicly, prepare derivative works,
12+
# distribute, and otherwise use Python alone or in any derivative version,
13+
# provided, however, that PSF's License Agreement and PSF's notice of copyright,
14+
# i.e., "Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
15+
# 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Python Software Foundation;
16+
# All Rights Reserved" are retained in Python alone or in any derivative version
17+
# prepared by Licensee.
18+
#
19+
# 3. In the event Licensee prepares a derivative work that is based on
20+
# or incorporates Python or any part thereof, and wants to make
21+
# the derivative work available to others as provided herein, then
22+
# Licensee hereby agrees to include in any such work a brief summary of
23+
# the changes made to Python.
24+
#
25+
# 4. PSF is making Python available to Licensee on an "AS IS"
26+
# basis. PSF MAKES NO REPRESENTATIONS OR WARRANTIES, EXPRESS OR
27+
# IMPLIED. BY WAY OF EXAMPLE, BUT NOT LIMITATION, PSF MAKES NO AND
28+
# DISCLAIMS ANY REPRESENTATION OR WARRANTY OF MERCHANTABILITY OR FITNESS
29+
# FOR ANY PARTICULAR PURPOSE OR THAT THE USE OF PYTHON WILL NOT
30+
# INFRINGE ANY THIRD PARTY RIGHTS.
31+
#
32+
# 5. PSF SHALL NOT BE LIABLE TO LICENSEE OR ANY OTHER USERS OF PYTHON
33+
# FOR ANY INCIDENTAL, SPECIAL, OR CONSEQUENTIAL DAMAGES OR LOSS AS
34+
# A RESULT OF MODIFYING, DISTRIBUTING, OR OTHERWISE USING PYTHON,
35+
# OR ANY DERIVATIVE THEREOF, EVEN IF ADVISED OF THE POSSIBILITY THEREOF.
36+
#
37+
# 6. This License Agreement will automatically terminate upon a material
38+
# breach of its terms and conditions.
39+
#
40+
# 7. Nothing in this License Agreement shall be deemed to create any
41+
# relationship of agency, partnership, or joint venture between PSF and
42+
# Licensee. This License Agreement does not grant permission to use PSF
43+
# trademarks or trade name in a trademark sense to endorse or promote
44+
# products or services of Licensee, or any third party.
45+
#
46+
# 8. By copying, installing or otherwise using Python, Licensee
47+
# agrees to be bound by the terms and conditions of this License
48+
# Agreement.
49+
#
50+
# type: ignore
51+
52+
import itertools
53+
import os
54+
import threading
55+
import warnings
56+
from asyncio import AbstractChildWatcher, events
57+
from asyncio.log import logger
58+
59+
60+
class ThreadedChildWatcher(AbstractChildWatcher):
61+
"""Threaded child watcher implementation.
62+
The watcher uses a thread per process
63+
for waiting for the process finish.
64+
It doesn't require subscription on POSIX signal
65+
but a thread creation is not free.
66+
The watcher has O(1) complexity, its performance doesn't depend
67+
on amount of spawn processes.
68+
"""
69+
70+
def __init__(self):
71+
self._pid_counter = itertools.count(0)
72+
self._threads = {}
73+
74+
def is_active(self):
75+
return True
76+
77+
def close(self):
78+
self._join_threads()
79+
80+
def _join_threads(self):
81+
"""Internal: Join all non-daemon threads"""
82+
threads = [
83+
thread
84+
for thread in list(self._threads.values())
85+
if thread.is_alive() and not thread.daemon
86+
]
87+
for thread in threads:
88+
thread.join()
89+
90+
def __enter__(self):
91+
return self
92+
93+
def __exit__(self, exc_type, exc_val, exc_tb):
94+
pass
95+
96+
def __del__(self, _warn=warnings.warn):
97+
threads = [
98+
thread for thread in list(self._threads.values()) if thread.is_alive()
99+
]
100+
if threads:
101+
_warn(
102+
f"{self.__class__} has registered but not finished child processes",
103+
ResourceWarning,
104+
source=self,
105+
)
106+
107+
def add_child_handler(self, pid, callback, *args):
108+
loop = events.get_running_loop()
109+
thread = threading.Thread(
110+
target=self._do_waitpid,
111+
name=f"waitpid-{next(self._pid_counter)}",
112+
args=(loop, pid, callback, args),
113+
daemon=True,
114+
)
115+
self._threads[pid] = thread
116+
thread.start()
117+
118+
def remove_child_handler(self, pid):
119+
# asyncio never calls remove_child_handler() !!!
120+
# The method is no-op but is implemented because
121+
# abstract base classe requires it
122+
return True
123+
124+
def attach_loop(self, loop):
125+
pass
126+
127+
def _do_waitpid(self, loop, expected_pid, callback, args):
128+
assert expected_pid > 0
129+
130+
try:
131+
pid, status = os.waitpid(expected_pid, 0)
132+
except ChildProcessError:
133+
# The child process is already reaped
134+
# (may happen if waitpid() is called elsewhere).
135+
pid = expected_pid
136+
returncode = 255
137+
logger.warning(
138+
"Unknown child process pid %d, will report returncode 255", pid
139+
)
140+
else:
141+
returncode = _compute_returncode(status)
142+
if loop.get_debug():
143+
logger.debug(
144+
"process %s exited with returncode %s", expected_pid, returncode
145+
)
146+
147+
if loop.is_closed():
148+
logger.warning("Loop %r that handles pid %r is closed", loop, pid)
149+
else:
150+
loop.call_soon_threadsafe(callback, pid, returncode, *args)
151+
152+
self._threads.pop(expected_pid)
153+
154+
155+
def _compute_returncode(status):
156+
if os.WIFSIGNALED(status):
157+
# The child process died because of a signal.
158+
return -os.WTERMSIG(status)
159+
elif os.WIFEXITED(status):
160+
# The child process exited (e.g sys.exit()).
161+
return os.WEXITSTATUS(status)
162+
else:
163+
# The child exited, but we don't understand its status.
164+
# This shouldn't happen, but if it does, let's just
165+
# return that status; perhaps that helps debug it.
166+
return status

0 commit comments

Comments
 (0)