Skip to content

Initialize/Finalize Python multiple time and import datetime each time lead to a memory corruption #118608

Description

@neonene

Crash report

What happened?

Example on Windows (debug build):

# Taken from _ctypes issue: #116467
import subprocess
program = r"PCbuild\amd64\_testembed_d.exe"
cmd = [program, "test_repeated_init_exec", "import datetime"]   # or _datetime

for i in range(1, 11):
    print(f"   == Process #{i} ===")
    proc = subprocess.run(cmd)
    exitcode = proc.returncode
    print(f"=> exitcode {exitcode}")
    if exitcode:
        break
    print()

Output (7bbce38: CI failures with a similar test):

Running Debug|x64 interpreter...
   == Process #1 ===
--- Loop #1 ---
--- Loop #2 ---
Assertion failed: PyUnicode_CheckExact(ep_key), file C:\a\Objects\dictobject.c, line 1119
=> exitcode 3

It is an expected error on a debug build, which comes form _PyUnicode_ClearInterned() in unicodeobject.c of the commit ea2c001:

void
_PyUnicode_ClearInterned(PyInterpreterState *interp)
{
    ...
    /* TODO:
     * Currently, the runtime is not able to guarantee that it can exit without
     * allocations that carry over to a future initialization of Python within
     * the same process. i.e:
     *   ./python -X showrefcount -c 'import itertools'
     *   [237 refs, 237 blocks]
     *
     * Therefore, this should remain disabled for until there is a strict guarantee
     * that no memory will be left after `Py_Finalize`.
     */
#ifdef Py_DEBUG
    ...

Currently, release builds avoid the datetime crash by leaking all interned strings. Both the crash and the leak should be fixed before shipping 3.13 final.


This dedicated issue is requested in:

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Linux, macOS, Windows

Linked PRs

Activity

  1. added
    type-crashA hard crash of the interpreter, possibly with a core dump
    on May 5, 2024
  2. vstinner commented on May 6, 2024

    @vstinner
    Member

    Which parts of the code use interned strings?

  3. neonene commented on May 6, 2024

    @neonene
    ContributorAuthor

    If you mean _datetimemodule.c, _datetime_exec() is a place where assertion errors occur, trying to access dict keys (invalid interned strings).

    PyObject *d = PyDateTime_DeltaType.tp_dict;
    DATETIME_ADD_MACRO(d, "resolution", new_delta(0, 0, 1, 0));
    DATETIME_ADD_MACRO(d, "min", new_delta(-MAX_DELTA_DAYS, 0, 0, 0));
    DATETIME_ADD_MACRO(d, "max",
    new_delta(MAX_DELTA_DAYS, 24*3600-1, 1000000-1, 0));
    /* date values */
    d = PyDateTime_DateType.tp_dict;
    DATETIME_ADD_MACRO(d, "min", new_date(1, 1, 1));
    DATETIME_ADD_MACRO(d, "max", new_date(MAXYEAR, 12, 31));
    DATETIME_ADD_MACRO(d, "resolution", new_delta(1, 0, 0, 0));
    /* time values */
    d = PyDateTime_TimeType.tp_dict;
    DATETIME_ADD_MACRO(d, "min", new_time(0, 0, 0, 0, Py_None, 0));
    DATETIME_ADD_MACRO(d, "max", new_time(23, 59, 59, 999999, Py_None, 0));
    DATETIME_ADD_MACRO(d, "resolution", new_delta(0, 0, 1, 0));
    /* datetime values */
    d = PyDateTime_DateTimeType.tp_dict;
    DATETIME_ADD_MACRO(d, "min",
    new_datetime(1, 1, 1, 0, 0, 0, 0, Py_None, 0));
    DATETIME_ADD_MACRO(d, "max", new_datetime(MAXYEAR, 12, 31, 23, 59, 59,
    999999, Py_None, 0));
    DATETIME_ADD_MACRO(d, "resolution", new_delta(0, 0, 1, 0));
    datetime_state *st = STATIC_STATE();
    if (init_state(st) < 0) {
    goto error;
    }
    /* timezone values */
    d = PyDateTime_TimeZoneType.tp_dict;
    if (PyDict_SetItemString(d, "utc", st->utc) < 0) {
    goto error;
    }
    /* bpo-37642: These attributes are rounded to the nearest minute for backwards
    * compatibility, even though the constructor will accept a wider range of
    * values. This may change in the future.*/
    /* -23:59 */
    PyObject *min = create_timezone_from_delta(-1, 60, 0, 1);
    DATETIME_ADD_MACRO(d, "min", min);
    /* +23:59 */
    PyObject *max = create_timezone_from_delta(0, (23 * 60 + 59) * 60, 0, 0);
    DATETIME_ADD_MACRO(d, "max", max);

  4. vstinner commented on May 7, 2024

    @vstinner
    Member

    With a debug build, I confirm that an assertion fails at the first PyDict_SetItemString() call on PyDateTime_DeltaType.tp_dict, on the second _datetime_exec() call.

    PyObject *d = PyDateTime_DeltaType.tp_dict;
    DATETIME_ADD_MACRO(d, "resolution", new_delta(0, 0, 1, 0));

    For me, the root issue is that datetime doesn't use heap types but static types.

    The symptom is that PyDict_SetItemString() calls PyUnicode_InternInPlace() and so stores interned strings, but interned strings are cleared at Python exit: in Py_Finalize().

    Interned strings are just one symptom, but I expect other side effects of reusing static types between two Python executions.

    IMO only isolated extensions are safe to be used with the "sub-interpreter" case:

    • Use multiple interpreters using sub-interpreters
    • Or init/finalize Python multiple times which is a similar but different case
  5. vstinner commented on May 7, 2024

    @vstinner
    Member

    Initialize/Finalize Python multiple time and import datetime each time lead to a memory corruption

    That's one way to see the issue. Another way to see is that Python doesn't support datetime with "sub-interpreters" (see my previous message). This issue is about supporting sub-interpreters in datetime :-)

  6. neonene commented on May 7, 2024

    @neonene
    ContributorAuthor

    For me, the root issue is that datetime doesn't use heap types but static types.

    Yes, but you prevented me from the heap types conversion at #103092 (comment).

    Interned strings are just one symptom, but I expect other side effects of reusing static types between two Python executions.

    The main and non-isolated subinterpreters share the same module, which avoids redundant initializations. And many objects in a static type are carried over except interned strings. (3.12 needs PR #118618 to carry over static types)

    What case are you expecting?

    IMO only isolated extensions are safe to be used with the "sub-interpreter" case:

    Isolated sub-interpreters are not allowed to load a single-phase init module.

  7. neonene commented on May 7, 2024

    @neonene
    ContributorAuthor

    Isolated sub-interpreters are not allowed to load a single-phase init module.

    See: PEP 554, C-API

    That's also why PEP-687 needs to be completed.

  8. vstinner commented on May 7, 2024

    @vstinner
    Member

    Yes, but you prevented me from the heap types conversion at #103092 (comment).

    Right, I am afraid of regression. We should do the conversion in a specific order, step by step, to limit risk and be able to rollback if everything goes wrong.

    What case are you expecting?

    Objects must not be shared between interpreters, including types. Each interpreter should have its own types to prevent any race condition / concurrent accesses, especially when each interpreter has its own GIL. I don't recall details.

    It doesn't matter much. We both agree to isolate the extension.

    My proposed plan: #117398 (comment)

  9. neonene commented on May 7, 2024

    @neonene
    ContributorAuthor

    Can the isolation be backported to 3.13? Your concern for the sub-interpreters is another specific issue, which is not confirmed on release builds right now.

  10. vstinner commented on May 8, 2024

    @vstinner
    Member

    Can the isolation be backported to 3.13?

    I prefer to not backport such large change.

    For 3.12 and 3.13, we may just stop clearing interned strings at exit, and only do that at Py_Main() exit.

  11. neonene commented on May 8, 2024

    @neonene
    ContributorAuthor

    For 3.12 and 3.13, we may just stop clearing interned strings at exit, and only do that at Py_Main() exit.

    See also #113993

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-crashA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions