8000 gh-116773: Ensure overlapped objects on Windows are not deallocated t… · python/cpython@fc45998 · GitHub
[go: up one dir, main page]
Skip to content

Commit fc45998

Browse files
jkriegshauserjkriegshauser
authored
gh-116773: Ensure overlapped objects on Windows are not deallocated too early by asyncio (GH-116774)
1 parent 519b2ae commit fc45998

File tree

4 files changed

+71
-12
lines changed

4 files changed

+71
-12
lines changed

‎Lib/asyncio/windows_events.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,13 @@ def _run_forever_cleanup(self):
324324
if self._self_reading_future is not None:
325325
ov = self._self_reading_future._ov
326326
self._self_reading_future.cancel()
327-
# self_reading_future was just cancelled so if it hasn't been
328-
# finished yet, it never will be (it's possible that it has
329-
# already finished and its callback is waiting in the queue,
330-
# where it could still happen if the event loop is restarted).
331-
# Unregister it otherwise IocpProactor.close will wait for it
332-
# forever
333-
if ov is not None:
327+
# self_reading_future always uses IOCP, so even though it's
328+
# been cancelled, we need to make sure that the IOCP message
329+
# is received so that the kernel is not holding on to the
330+
# memory, possibly causing memory corruption later. Only
331+
# unregister it if IO is complete in all respects. Otherwise
332+
# we need another _poll() later to complete the IO.
333+
if ov is not None and not ov.pending:
334334
self._proactor._unregister(ov)
335335
self._self_reading_future = None
336336

‎Lib/test/test_asyncio/test_windows_events.py

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,23 @@ def data_received(self, data):
3636
self.trans.close()
3737

3838

39-
class ProactorLoopCtrlC(test_utils.TestCase):
39+
class WindowsEventsTestCase(test_utils.TestCase):
40+
def _unraisablehook(self, unraisable):
41+
# Storing unraisable.object can resurrect an object which is being
42+
# finalized. Storing unraisable.exc_value creates a reference cycle.
43+
self._unraisable = unraisable
44+
print(unraisable)
45+
46+
def setUp(self):
47+
self._prev_unraisablehook = sys.unraisablehook
48+
self._unraisable = None
49+
sys.unraisablehook = self._unraisablehook
50+
51+
def tearDown(self):
52+
sys.unraisablehook = self._prev_unraisablehook
53+
self.assertIsNone(self._unraisable)
54+
55+
class ProactorLoopCtrlC(WindowsEventsTestCase):
4056

4157
def test_ctrl_c(self):
4258

@@ -58,7 +74,7 @@ def SIGINT_after_delay():
5874
thread.join()
5975

6076

61-
class ProactorMultithreading(test_utils.TestCase):
77+
class ProactorMultithreading(WindowsEventsTestCase):
6278
def test_run_from_nonmain_thread(self):
6379
finished = False
6480

@@ -79,7 +95,7 @@ def func():
7995
self.assertTrue(finished)
8096

8197

82-
class ProactorTests(test_utils.TestCase):
98+
class ProactorTests(WindowsEventsTestCase):
8399

84100
def setUp(self):
85101
super().setUp()
@@ -283,8 +299,32 @@ async def probe():
283299

284300
return "done"
285301

286-
287-
class WinPolicyTests(test_utils.TestCase):
302+
def test_loop_restart(self):
303+
# We're fishing for the "RuntimeError: <_overlapped.Overlapped object at XXX>
304+
# still has pending operation at deallocation, the process may crash" error
305+
stop = threading.Event()
306+
def threadMain():
307+
while not stop.is_set():
308+
self.loop.call_soon_threadsafe(lambda: None)
309+
time.sleep(0.01)
310+
thr = threading.Thread(target=threadMain)
311+
312+
# In 10 60-second runs of this test prior to the fix:
313+
# time in seconds until failure: (none), 15.0, 6.4, (none), 7.6, 8.3, 1.7, 22.2, 23.5, 8.3
314+
# 10 seconds had a 50% failure rate but longer would be more costly
315+
end_time = time.time() + 10 # Run for 10 seconds
316+
self.loop.call_soon(thr.start)
317+
while not self._unraisable: # Stop if we got an unraisable exc
318+
self.loop.stop()
319+
self.loop.run_forever()
320+
if time.time() >= end_time:
321+
break
322+
323+
stop.set()
324+
thr.join()
325+
326+
327+
class WinPolicyTests(WindowsEventsTestCase):
288328

289329
def test_selector_win_policy(self):
290330
async def main():

‎Misc/NEWS.d/next/Windows/2024-03-14-01-58-22.gh-issue-116773.H2UldY.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix instances of ``<_overlapped.Overlapped object at 0xXXX> still has pending operation at deallocation, the process may crash``.

‎Modules/overlapped.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,24 @@ Overlapped_dealloc(OverlappedObject *self)
723723
if (!HasOverlappedIoCompleted(&self->overlapped) &&
724724
self->type != TYPE_NOT_STARTED)
725725
{
726+
// NOTE: We should not get here, if we do then something is wrong in
727+
// the IocpProactor or ProactorEventLoop. Since everything uses IOCP if
728+
// the overlapped IO hasn't completed yet then we should not be
729+
// deallocating!
730+
//
731+
// The problem is likely that this OverlappedObject was removed from
732+
// the IocpProactor._cache before it was complete. The _cache holds a
733+
// reference while IO is pending so that it does not get deallocated
734+
// while the kernel has retained the OVERLAPPED structure.
735+
//
736+
// CancelIoEx (likely called from self.cancel()) may have successfully
737+
// completed, but the OVERLAPPED is still in use until either
738+
// HasOverlappedIoCompleted() is true or GetQueuedCompletionStatus has
739+
// returned this OVERLAPPED object.
740+
//
741+
// NOTE: Waiting when IOCP is in use can hang indefinitely, but this
742+
// CancelIoEx is superfluous in that self.cancel() was already called,
743+
// so I've only ever seen this return FALSE with GLE=ERROR_NOT_FOUND
726744
Py_BEGIN_ALLOW_THREADS
727745
if (CancelIoEx(self->handle, &self->overlapped))
728746
wait = TRUE;

0 commit comments

Comments
 (0)
0