8000 [3.12] gh-116773: Fix overlapped memory corruption crash (GH-116774) … · python/cpython@d69c515 · GitHub
[go: up one dir, main page]
Skip to content

Commit d69c515

Browse files
jkriegshauserjkriegshauser
authored
[3.12] gh-116773: Fix overlapped memory corruption crash (GH-116774) (#117078)
Manual backport of gh-116774.
1 parent 8e00ead commit d69c515

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

‎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
@@ -719,6 +719,24 @@ Overlapped_dealloc(OverlappedObject *self)
719719
if (!HasOverlappedIoCompleted(&self->overlapped) &&
720720
self->type != TYPE_NOT_STARTED)
721721
{
722+
// NOTE: We should not get here, if we do then something is wrong in
723+
// the IocpProactor or ProactorEventLoop. Since everything uses IOCP if
724+
// the overlapped IO hasn't completed yet then we should not be
725+
// deallocating!
726+
//
727+
// The problem is likely that this OverlappedObject was removed from
728+
// the IocpProactor._cache before it was complete. The _cache holds a
729+
// reference while IO is pending so that it does not get deallocated
730+
// while the kernel has retained the OVERLAPPED structure.
731+
//
732+
// CancelIoEx (likely called from self.cancel()) may have successfully
733+
// completed, but the OVERLAPPED is still in use until either
734+
// HasOverlappedIoCompleted() is true or GetQueuedCompletionStatus has
735+
// returned this OVERLAPPED object.
736+
//
737+
// NOTE: Waiting when IOCP is in use can hang indefinitely, but this
738+
// CancelIoEx is superfluous in that self.cancel() was already called,
739+
// so I've only ever seen this return FALSE with GLE=ERROR_NOT_FOUND
722740
Py_BEGIN_ALLOW_THREADS
723741
if (CancelIoEx(self->handle, &self->overlapped))
724742
wait = TRUE;

0 commit comments

Comments
 (0)
0