Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault with asyncio.base_events.BaseEventLoop when passed a small float to set_debug. #126881

Closed
devdanzin opened this issue Nov 15, 2024 · 15 comments
Assignees
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@devdanzin
Copy link
Contributor

devdanzin commented Nov 15, 2024

Crash report

What happened?

It's possible to segfault the interpreter by passing a small float as the enabled parameter of asyncio.base_events.BaseEventLoop.set_debug():

import asyncio.base_events

obj = asyncio.base_events.BaseEventLoop()
obj.set_debug(0.0005)
obj._run_forever_setup()

Found using fusil by @vstinner.

CPython versions tested on:

3.13, 3.14, CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.14.0a1+ experimental free-threading build (heads/main-dirty:54c63a32d0, Nov 8 2024, 20:16:36) [GCC 11.4.0]

Linked PRs

@devdanzin devdanzin added the type-crash A hard crash of the interpreter, possibly with a core dump label Nov 15, 2024
@picnixz picnixz added topic-asyncio 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Nov 16, 2024
@github-project-automation github-project-automation bot moved this to Todo in asyncio Nov 16, 2024
@picnixz picnixz added extension-modules C modules in the Modules dir topic-free-threading labels Nov 16, 2024
@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

I'm assuming that this only happens on the free-threaded build, or?

@devdanzin
Copy link
Contributor Author

No, it segfaults on normal build too, sorry for not mentioning it.

@picnixz picnixz added 3.12 bugs and security fixes and removed topic-free-threading labels Nov 16, 2024
@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

I'll have a look at this one.

@picnixz picnixz self-assigned this Nov 16, 2024
@picnixz picnixz added the stdlib Python modules in the Lib dir label Nov 16, 2024
@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

FTR: the interpreter crashes upon exit (at least on my machine)

Checked 112 modules (34 built-in, 77 shared, 1 n/a on linux-x86_64, 0 disabled, 0 missing, 0 failed on import)
Python 3.14.0a1+ (heads/fix/asyncio-set-debug-126881-dirty:08f98f4576f, Nov 16 2024, 10:29:31) [GCC 7.5.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio.base_events
...
... obj = asyncio.base_events.BaseEventLoop()
... obj.set_debug(0.0005)
... obj._run_forever_setup()
...
>>>
Segmentation fault (core dumped)

@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

Something is happening in the __del__ method:

    def __del__(self, _warn=warnings.warn):
        if not self.is_closed():
            _warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)
            if not self.is_running():
                self.close()

Now, if I just put a print just before if not self.is_closed(), no crash happens but the print is not printed. I think the moment close is being called, the _debug attribute is already gone, hence causing an segfault. Actually, changing to:

    def __del__(self, _warn=warnings.warn, _str=str, _stdout=sys.stdout):
        _stdout.write("no!!!" + _str(self) + "\n")
        if not self.is_closed():
            _warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)
            if not self.is_running():
                self.close()

The line _stdout.write is NOT printed but if I change _str(self) to _str(1), it is printed.

@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

The following patch seems to work though I wonder why. I think it's

diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py
index 1c7ffe37c75..d7d64162108 100644
--- a/Lib/asyncio/base_events.py
+++ b/Lib/asyncio/base_events.py
@@ -2052,8 +2052,9 @@ def get_debug(self):
         return self._debug

     def set_debug(self, enabled):
-        self._debug = enabled
+        self._debug = enabled = bool(enabled)

         if self.is_running():
             self.call_soon_threadsafe(self._set_coroutine_origin_tracking, enabled)

By the way, the segfault happens when cleaning up _frozen_importlib (or maybe it's # destroy traceback that crashes):

# destroy _struct
# clear sys.meta_path
# clear sys.modules
# destroy _frozen_importlib
Segmentation fault (core dumped)

@devdanzin
Copy link
Contributor Author

devdanzin commented Nov 16, 2024

The backtrace is:

#0  0x00007ffff7bc9f51 in mult (a=0x7ffff7f19338 <_PyRuntime+118936>, b=0x0) at Python/dtoa.c:618
#1  0x00007ffff7bca2dc in pow5mult (b=0x7ffff7f19338 <_PyRuntime+118936>, k=1) at Python/dtoa.c:702
#2  0x00007ffff7bcdede in _Py_dg_dtoa (dd=0.00050000000000000001, mode=0, ndigits=0, decpt=0x7fffffff956c,
    sign=0x7fffffff9570, rve=0x7fffffff9580) at Python/dtoa.c:2550
#3  0x00007ffff7bc82e0 in format_float_short (d=0.00050000000000000001, format_code=114 'r', mode=0,
    precision=0, always_add_sign=0, add_dot_0_if_integer=2, use_alt_formatting=0, no_negative_zero=0,
    float_strings=0x7ffff7e6fab0 <lc_float_strings>, type=0x0) at Python/pystrtod.c:986
#4  0x00007ffff7bc8b3d in PyOS_double_to_string (val=0.00050000000000000001, format_code=114 'r',
    precision=0, flags=2, type=0x0) at Python/pystrtod.c:1279
#5  0x00007ffff794ca67 in float_repr (v=0x7ffff73ac970) at Objects/floatobject.c:380
#6  0x00007ffff79ecc50 in object_str (self=0x7ffff73ac970) at Objects/typeobject.c:6215
#7  0x00007ffff799fa89 in PyObject_Str (v=0x7ffff73ac970) at Objects/object.c:742

That, along with the fact that using 0.5 instead of 0.0005 doesn't segfault, points to the repr of _debug in the ResourceWarning on interpreter shutdown being the cause of the segfault:

/Lib/asyncio/base_events.py:759: ResourceWarning: unclosed event loop <BaseEventLoop running=False closed=False debug=0.0005>

Commenting out bool(enabled) (where enabled is self._debug) here avoids the segfault:

def _set_coroutine_origin_tracking(self, enabled):
if bool(enabled) == bool(self._coroutine_origin_tracking_enabled):
return

@picnixz
Copy link
Contributor

picnixz commented Nov 16, 2024

Maybe it's an issue with bool at finalization time?

EDIT: I still have a crash even if I comment the above line.

cc @ncoghlan (I don't really know who to ask for this).

@devdanzin
Copy link
Contributor Author

I'm guessing at the time we try to repr that float, interp->dtoa.p5s has been freed/set to NULL/contains NULL (sorry, my C is weak):

#0  0x00007ffff7bc9f51 in mult (a=0x7ffff7f19338 <_PyRuntime+118936>, b=0x0) at Python/dtoa.c:618

This would mean mult has been called with b=0x0 from line 702 here:

cpython/Python/dtoa.c

Lines 695 to 702 in 2313f84

PyInterpreterState *interp = _PyInterpreterState_GET();
p5s = interp->dtoa.p5s;
for(;;) {
assert(p5s != interp->dtoa.p5s + Bigint_Pow5size);
p5 = *p5s;
p5s++;
if (k & 1) {
b1 = mult(b, p5);

Now, why that would segfault with 0.0005 but not 0.5 I have no idea.

@kumaraditya303
Copy link
Contributor

I am investigating this, will have a fix soon.

@kumaraditya303 kumaraditya303 self-assigned this Nov 16, 2024
@picnixz

This comment was marked as resolved.

@picnixz picnixz removed their assignment Nov 16, 2024
@kumaraditya303 kumaraditya303 added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed topic-asyncio stdlib Python modules in the Lib dir extension-modules C modules in the Modules dir labels Nov 16, 2024
@kumaraditya303
Copy link
Contributor

#126904 fixes this

@vstinner
Copy link
Member

self._debug = enabled = bool(enabled)

IMO it's also worth it to fix asyncio.

@picnixz
Copy link
Contributor

picnixz commented Nov 18, 2024

I'll reopen it once I'm back from my travels if you think it's worth it!

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 29, 2024
(cherry picked from commit 762c603)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@kumaraditya303
Copy link
Contributor

Fixed by #126904

@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Nov 29, 2024
vstinner pushed a commit that referenced this issue Nov 29, 2024
gh-126881: fix finalization of dtoa state (GH-126904)
(cherry picked from commit 762c603)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
Projects
Status: Done
Development

No branches or pull requests

4 participants