classification
Title: Fix possible SIGSGV when asyncio.Future is created in __del__
Type: crash Stage: resolved
Components: asyncio Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, miss-islington, ned.deily, pbasista, serhiy.storchaka, vstinner, yselivanov
Priority: release blocker Keywords: patch

Created on 2018-05-23 18:55 by yselivanov, last changed 2018-05-29 15:16 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
uvloop-test.gdb-backtrace.txt pbasista, 2018-05-28 14:54 GDB backtrace showing the assertion failure while using uvloop
Pull Requests
URL Status Linked Edit
PR 7080 merged yselivanov, 2018-05-23 18:59
PR 7151 merged miss-islington, 2018-05-28 15:12
PR 7152 closed miss-islington, 2018-05-28 15:12
Messages (12)
msg317437 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-23 18:55
Originally reported in https://github.com/MagicStack/uvloop/issues/143

Future.__init__ shouldn't try to capture the current traceback if the interpreter is being finalized.
msg317440 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-23 19:55
Is the problem because traceback_extract_stack is NULL?

But this will not fix the problem completely. For example calling repr() on a future can crash because of asyncio_future_repr_info_func == NULL. And many other operations use globals cleared in module_free().
msg317441 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-23 19:56
> Is the problem because traceback_extract_stack is NULL?

It's not NULL, it's just a broken reference at that point.
msg317444 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-23 20:02
Then we should find what callable is NULL and fix the place where it is called. _PyObject_FastCallDict() should never be called with NULL.
msg317602 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-24 18:28
> Then we should find what callable is NULL and fix the place where it is called. _PyObject_FastCallDict() should never be called with NULL.

My understanding is that the interpreter is being shutdown and half of the objects are freed. We're still holding a reference to *something* in Python space and try calling it.  The obvious fix for that is simply avoid capturing tracebacks if a Future object is created during finalization of the interpreter (it's pointless to capture it anyways at that point).
msg317658 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-25 06:44
It is okay if the fact that half of the objects are freed leads to raising exceptions. But an assertion failure is sign of bugs in Python core or the _asyncio module. PR 7080 removes one way of exposing these bugs, but bugs itself still are here, and there may be other ways of triggering a crash.

Can this crash be reproduced without uvloop? What is the backtrace of the assertion failure?
msg317858 - (view) Author: Peter Bašista (pbasista) Date: 2018-05-28 14:54
> Can this crash be reproduced without uvloop?
I am not aware of a way to reproduce this without uvloop.

> What is the backtrace of the assertion failure?
Without uvloop, the following exception is raised:

Exception ignored in: <bound method LoopWrapper.__del__ of <__main__.LoopWrapper object at 0x7fe14ec569b0>>
Traceback (most recent call last):
  File "uvloop_test.py", line 13, in __del__
  File "/usr/lib/python3.6/asyncio/base_events.py", line 276, in create_future
AttributeError: 'NoneType' object has no attribute 'Future'

With uvloop and with Python compiled with --with-pydebug configure option, the failed assertion is mentioned in the original uvloop bug report. In particular, this assertion fails:

python3.6-dbg: ../Objects/abstract.c:2300: _PyObject_FastCallDict: Assertion `func != NULL' failed. 

A full GDB backtrace is attached.
msg317863 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-28 15:11
There's nothing specific here to uvloop except the fact that it was compiled with Cython, so module finalization/GC pattern is slightly different to that of vanilla asyncio.

Serhiy, I'm merging this PR since I'd like to avoid having segfaults in 3.7.0.  Feel free to debug this further.
msg317864 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-28 15:11
New changeset 35230d08e09de4e2e52658d5cb09e5b0ca965418 by Yury Selivanov in branch 'master':
bpo-33623: Fix possible SIGSGV when asyncio.Future is created in __del__ (#7080)
https://github.com/python/cpython/commit/35230d08e09de4e2e52658d5cb09e5b0ca965418
msg317868 - (view) Author: miss-islington (miss-islington) Date: 2018-05-28 15:28
New changeset 51d0a2c8ddcb9f58d71ff0a62be4e31a1af3f139 by Miss Islington (bot) in branch '3.7':
bpo-33623: Fix possible SIGSGV when asyncio.Future is created in __del__ (GH-7080)
https://github.com/python/cpython/commit/51d0a2c8ddcb9f58d71ff0a62be4e31a1af3f139
msg317930 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-28 21:57
Would be great to merge this in 3.7.0.  The change is super safe to merge.
msg318026 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-29 13:52
> Would be great to merge this in 3.7.0.  The change is super safe to merge.

The 3.7rc1 is not supposed to be tagged on the current 3.7 branch?
History
Date User Action Args
2018-05-29 15:16:35yselivanovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-05-29 13:52:12vstinnersetnosy: + vstinner
messages: + msg318026
2018-05-28 21:57:06yselivanovsetpriority: normal -> release blocker
nosy: + ned.deily
messages: + msg317930

2018-05-28 15:28:14miss-islingtonsetnosy: + miss-islington
messages: + msg317868
2018-05-28 15:12:43miss-islingtonsetpull_requests: + pull_request6788
2018-05-28 15:12:01miss-islingtonsetpull_requests: + pull_request6787
2018-05-28 15:11:34yselivanovsetmessages: + msg317864
2018-05-28 15:11:14yselivanovsetmessages: + msg317863
2018-05-28 14:54:17pbasistasetfiles: + uvloop-test.gdb-backtrace.txt
nosy: + pbasista
messages: + msg317858

2018-05-25 06:44:08serhiy.storchakasetmessages: + msg317658
2018-05-24 18:28:24yselivanovsetmessages: + msg317602
2018-05-23 20:02:25serhiy.storchakasetmessages: + msg317444
2018-05-23 19:56:56yselivanovsetmessages: + msg317441
2018-05-23 19:55:36serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg317440
2018-05-23 19:21:39yselivanovsettype: crash
2018-05-23 19:21:25yselivanovsetnosy: + asvetlov
components: + asyncio
2018-05-23 18:59:21yselivanovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6710
2018-05-23 18:55:46yselivanovcreate