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

Objects/typeobject.c:143: managed_static_type_index_get: Assertion managed_static_type_index_is_set(self)' failed.` in 3.13.0b2+, with freezegun #120161

Closed
mgorny opened this issue Jun 6, 2024 · 11 comments
Assignees
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@mgorny
Copy link
Contributor

mgorny commented Jun 6, 2024

Crash report

What happened?

Starting with Python 3.13.0b2, importing freezegun causes the interpreter to crash on exit. I was able to reduce it into the following ugly quasi-standalone snippet:

import asyncio
import datetime
from typing import Type


class tzutc(datetime.tzinfo):
    pass


_EPOCHTZ = datetime.datetime(1970, 1, 1, tzinfo=tzutc())


class FakeDateMeta(type):
    def __instancecheck__(self, obj):
        return True


class FakeDate(datetime.date, metaclass=FakeDateMeta):
    pass


def pickle_fake_date(datetime_) -> Type[FakeDate]:
    # A pickle function for FakeDate
    return FakeDate
$ python -c 'import repro'
python: Objects/typeobject.c:143: managed_static_type_index_get: Assertion `managed_static_type_index_is_set(self)' failed.
Aborted (core dumped)

I've been able to reproduce it with 3.13.0b2, tip of 3.13 branch and tip of main branch.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

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

Python 3.14.0a0 (heads/main:fd104dfcb83, Jun 6 2024, 16:55:57) [GCC 14.1.1 20240516]

Linked PRs

@mgorny mgorny added the type-crash A hard crash of the interpreter, possibly with a core dump label Jun 6, 2024
@mgorny
Copy link
Contributor Author

mgorny commented Jun 6, 2024

On 3.13, bisect points to:

commit e5fb3a2385809f6cbdba2061b40fecf5b234f549
Author:     Miss Islington (bot) <[email protected]>
AuthorDate: 2024-06-04 01:37:28 +0200
Commit:     GitHub <[email protected]>
CommitDate: 2024-06-04 01:37:28 +0200

    [3.13] gh-117398: Use Per-Interpreter State for the _datetime Static Types (gh-120009)
    
    We make use of the same mechanism that we use for the static builtin types.  This required a few tweaks.
    
    This change is the final piece needed to make _datetime support multiple interpreters.  I've updated the module slot accordingly.
    
    (cherry picked from commit 105f22ea46ac16866e6df18ebae2a8ba422b7f45, AKA gh-119929)
    
    Co-authored-by: Eric Snow <[email protected]>

 Include/internal/pycore_object.h                                        |   2 +-
 Include/internal/pycore_typeobject.h                                    |  48 ++++++++--
 Misc/NEWS.d/next/Library/2024-06-01-16-58-43.gh-issue-117398.kR0RW7.rst |   2 +
 Modules/_datetimemodule.c                                               | 197 ++++++++++++++++++++++++++++-----------
 Objects/exceptions.c                                                    |   2 +-
 Objects/object.c                                                        |   2 +-
 Objects/structseq.c                                                     |   2 +-
 Objects/typeobject.c                                                    | 278 +++++++++++++++++++++++++++++++++++++------------------
 Objects/unicodeobject.c                                                 |   6 +-
 Objects/weakrefobject.c                                                 |   2 +-
 Python/crossinterp_exceptions.h                                         |   4 +-
 Tools/c-analyzer/cpython/globals-to-fix.tsv                             |   1 +
 Tools/c-analyzer/cpython/ignored.tsv                                    |   1 +
 13 files changed, 381 insertions(+), 166 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Library/2024-06-01-16-58-43.gh-issue-117398.kR0RW7.rst

CC @ericsnowcurrently

@Eclips4
Copy link
Member

Eclips4 commented Jun 6, 2024

Thanks for the report!
Confirmed also on the current main branch.

@Eclips4 Eclips4 added 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Jun 6, 2024
@Eclips4
Copy link
Member

Eclips4 commented Jun 6, 2024

Here's backtrace:

#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007ffff7cf526e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007ffff7cd88ff in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007ffff7cd881b in __assert_fail_base (fmt=0x7ffff7e801e8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
    assertion=assertion@entry=0x5555558e26f0 "managed_static_type_index_is_set(self)", file=file@entry=0x5555558b1220 "Objects/typeobject.c",
    line=line@entry=143, function=function@entry=0x55555591bdb0 <__PRETTY_FUNCTION__.164> "managed_static_type_index_get") at ./assert/assert.c:94
#6  0x00007ffff7ceb507 in __assert_fail (assertion=assertion@entry=0x5555558e26f0 "managed_static_type_index_is_set(self)",
    file=file@entry=0x5555558b1220 "Objects/typeobject.c", line=line@entry=143,
    function=function@entry=0x55555591bdb0 <__PRETTY_FUNCTION__.164> "managed_static_type_index_get") at ./assert/assert.c:103
#7  0x00005555556f16b9 in managed_static_type_index_get (self=self@entry=0x7ffff77f0b80 <PyDateTime_TZInfoType>) at Objects/typeobject.c:143
#8  0x00005555556f16ce in managed_static_type_state_get (interp=0x555555b15c18 <_PyRuntime+101656>,
    self=self@entry=0x7ffff77f0b80 <PyDateTime_TZInfoType>) at Objects/typeobject.c:180
#9  0x00005555556f79cb in _PyStaticType_GetState (interp=<optimized out>, self=self@entry=0x7ffff77f0b80 <PyDateTime_TZInfoType>)
    at Objects/typeobject.c:197
#10 0x00005555556fa9bb in lookup_tp_subclasses (self=self@entry=0x7ffff77f0b80 <PyDateTime_TZInfoType>) at Objects/typeobject.c:554
#11 0x00005555556fcc7d in remove_subclass (base=0x7ffff77f0b80 <PyDateTime_TZInfoType>, type=type@entry=0x555555e3e940) at Objects/typeobject.c:8488
#12 0x00005555556fcdc3 in remove_all_subclasses (type=type@entry=0x555555e3e940, bases=bases@entry=0x7ffff7add9a0) at Objects/typeobject.c:8518
#13 0x00005555556fce28 in type_dealloc_common (type=type@entry=0x555555e3e940) at Objects/typeobject.c:5689
#14 0x00005555556fce74 in type_dealloc (self=0x555555e3e940) at Objects/typeobject.c:5802
#15 0x00005555556c93f2 in _Py_Dealloc (op=op@entry=0x555555e3e940) at Objects/object.c:2854
#16 0x00005555557d11c4 in Py_DECREF (filename=filename@entry=0x5555558b736d "Python/gc.c", lineno=lineno@entry=1113, op=op@entry=0x555555e3e940)
    at ./Include/refcount.h:351
#17 0x00005555557d1fd6 in delete_garbage (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>,
    gcstate=gcstate@entry=0x555555b17900 <_PyRuntime+109056>, collectable=collectable@entry=0x7fffffffd7b0,
    old=old@entry=0x555555b17948 <_PyRuntime+109128>) at Python/gc.c:1113
#18 0x00005555557d2289 in gc_collect_region (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>,
    from=from@entry=0x555555b17948 <_PyRuntime+109128>, to=to@entry=0x555555b17948 <_PyRuntime+109128>, untrack=untrack@entry=3,
    stats=stats@entry=0x7fffffffd880) at Python/gc.c:1564
#19 0x00005555557d278b in gc_collect_full (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>, stats=stats@entry=0x7fffffffd880)
    at Python/gc.c:1477
#20 0x00005555557d304b in _PyGC_Collect (tstate=0x555555b45468 <_PyRuntime+296296>, generation=generation@entry=2,
    reason=reason@entry=_Py_GC_REASON_SHUTDOWN) at Python/gc.c:1814
#21 0x00005555557d30d3 in _PyGC_CollectNoFail (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>) at Python/gc.c:1855
#22 0x000055555580ff13 in interpreter_clear (interp=0x555555b15c18 <_PyRuntime+101656>, tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>)
    at Python/pystate.c:872
#23 0x00005555558102b0 in _PyInterpreterState_Clear (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>) at Python/pystate.c:928
#24 0x000055555580b75d in finalize_interp_clear (tstate=tstate@entry=0x555555b45468 <_PyRuntime+296296>) at Python/pylifecycle.c:1867
#25 0x000055555580b8de in Py_FinalizeEx () at Python/pylifecycle.c:2107
#26 0x0000555555838ff1 in Py_RunMain () at Modules/main.c:720

@ericsnowcurrently ericsnowcurrently self-assigned this Jun 6, 2024
@ericsnowcurrently
Copy link
Member

I'll work on having a diagnosis (and a fix) today.

@mgorny, thanks for testing the betas!

@ericsnowcurrently
Copy link
Member

Here's more or less what's happening (the order is significant):

  1. the crashing code creates a tricky reference cycle involving a subclass of one of the _datetime static types
  2. interpreter finalization starts
  3. the _datetime static types are finalized (incl. __subclasses__)
  4. the reference cycle is resolved during the final GC collection, clearing the subclass and triggering the assert

At a high level, the solution is to make sure the reference cycle is cleared before the _datetime static types are finalized.
Notably, modules are also cleared before that same (final) GC collection and builtin static types are finalized after. We'll probably want to finalize the _datetime static types right before the builtin types. I'm going to explore a solution along these lines.

It's also worth noting that there could be a similar dynamic with the __dict__ of the builtin and _datetime static types. However, that's really only possible via the C-API.

CC @pablogsal (for GC)


FWIW, here are relevant details of the above reproducer are all necessary to reproduce the assertion:

  • import asyncio (never actually used)
  • import datetime but not _datetime
  • create an empty subclass of datetime.tzinfo
  • create a datetime.datetime instance, using an instance of the datetime.tzinfo subclass (the only reference to it)
  • create an empty subclass of datetime.date with a minimal custom metaclass
  • the custom metaclass implements __instancecheck__, which always returns True
  • implement a function that returns the date subclass (not an instance of the subclass)
  • add a return annotation to the function for the subclass

That combination of factors produces the ref cycle. Leaving out any one of those details means it does not crash. It isn't clear to me yet what asyncio and typing have to do with this, other than that they play a part in the ref cycle surviving until the end.

Here are some other related details:

  • datetime.date is the base class of datetime.datetime
  • the _datetime module may get destroyed earlier than expected, though not its contents
    • the datetime module doesn't hold a reference to _datetime, only to the objects bound to it when first imported
    • when first imported, the _datetime module doesn't hold a reference to any objects that directly/indirectly hold a reference back to the module
    • sys.modules should continue to hold a reference to _datetime

@ericsnowcurrently
Copy link
Member

Also, @mgorny, thanks for providing a simple-ish reproducer. There are a lot of non-obvious factors involved here so that has really helped in diagnosing the problem.

@ericsnowcurrently
Copy link
Member

I have a fix up. @mgorny, could you verify it solves the problem for you? Thanks!

@mgorny
Copy link
Contributor Author

mgorny commented Jun 7, 2024

Thanks. I've applied the patch (with some small rebasing) on top of 3.13.0b2, and I can confirm that the original issue with freezegun is gone. I'm going to keep running with it, and let you know if I notice any issues.

@neonene
Copy link
Contributor

neonene commented Jun 11, 2024

I can confirm the crash on Windows with the tip of 3.13 (64a61ca), but cannot with the main (9b8611e) PR).

UPDATE: 3.14 repro
import datetime
from _ast import Tuple
f = lambda: None
Tuple.dims = property(f, f)

class tzutc(datetime.tzinfo):
    pass

ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this issue Jun 12, 2024
@ericsnowcurrently
Copy link
Member

import datetime
from _ast import Tuple
f = lambda: None
Tuple.dims = property(f, f)

class tzutc(datetime.tzinfo):
    pass

@neonene, thanks for such a good reproducer. I've added it to the PR.

ericsnowcurrently added a commit that referenced this issue Jun 14, 2024
In gh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 14, 2024
In pythongh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.
(cherry picked from commit b2e71ff)

Co-authored-by: Eric Snow <[email protected]>
ericsnowcurrently added a commit that referenced this issue Jun 14, 2024
In gh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.

(cherry picked from commit b2e71ff, AKA gh-120182)

Co-authored-by: Eric Snow <[email protected]>
@ericsnowcurrently
Copy link
Member

I've landed the fix on the main and 3.13 branches.

mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
In pythongh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
In pythongh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
In pythongh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown.  However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection.  The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection.  To avoid the problem for _datetime, I have applied a similar approach here.

Also, credit goes to @mgorny and @neonene for the new tests.

FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting.  I'll circle back to the cleaner approach with a future change on the main branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

4 participants