Skip to content

Refcount issues in import #86160

Closed
Closed
Refcount issues in import#86160
Labels
3.10 (EOL)end of life3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

Description

@serhiy-storchaka
serhiy-storchaka
opened on Oct 10, 2020
BPO 41994
Nosy @brettcannon, @ncoghlan, @encukou, @ericsnowcurrently, @serhiy-storchaka, @indygreg
PRs
  • bpo-41994: Fix refcount issues in Python/import.c #22632
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-01-12.14:47:15.737>
created_at = <Date 2020-10-10.11:14:48.425>
labels = ['interpreter-core', '3.9', '3.10', 'performance']
title = 'Refcount issues in import'
updated_at = <Date 2021-09-28.01:02:00.403>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2021-09-28.01:02:00.403>
actor = 'indygreg'
assignee = 'none'
closed = True
closed_date = <Date 2021-01-12.14:47:15.737>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2020-10-10.11:14:48.425>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 41994
keywords = ['patch']
message_count = 7.0
messages = ['378384', '384943', '384947', '402688', '402696', '402698', '402755']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'ncoghlan', 'petr.viktorin', 'eric.snow', 'serhiy.storchaka', 'indygreg']
pr_nums = ['22632']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue41994'
versions = ['Python 3.9', 'Python 3.10']

    Activity

    1. serhiy-storchaka commented on Oct 10, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Oct 10, 2020
      MemberAuthor

      There is a reference leak in import_add_module(). PyDict_GetItemWithError() returns a borrowed reference, but PyObject_GetItem() return a non-borrowed reference. If sys.modules is not a dict, there is a reference leak. import_add_module() and several other function which return a borrowed reference should be made returning a non-borrowed reference, because there are no guaranties that general mapping keeps reference to value.

      It is still not guarantee correctness of PyImport_AddModuleObject().

    2. serhiy-storchaka
      added
      3.9 (EOL)end of life
      3.10 (EOL)end of life
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      performancePerformance or resource usage
      on Oct 10, 2020

    3. encukou commented on Jan 12, 2021

      @encukou
      encukou
      on Jan 12, 2021
      Member

      New changeset 4db8988 by Serhiy Storchaka in branch 'master':
      bpo-41994: Fix refcount issues in Python/import.c (GH-22632)
      4db8988

    4. serhiy-storchaka commented on Jan 12, 2021

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 12, 2021
      MemberAuthor

      Thank you Petr!

    5. serhiy-storchaka
      closed this as completedon Jan 12, 2021

    6. indygreg commented on Sep 27, 2021

      @indygreg
      indygregmannequin
      on Sep 27, 2021
      Mannequin

      While testing 3.10.0rc2, I noticed that commit 4db8988 removed _PyImport_FindExtensionObject() from libpython. This breaks both PyOxidizer and py2exe, which rely on this function for a custom implementation of Loader.create_module() to enable loading Windows DLLs from memory.

      It can probably be worked around. But I wanted to note it here in case it was an unwanted regression.

    7. encukou commented on Sep 27, 2021

      @encukou
      encukou
      on Sep 27, 2021
      Member

      Please don't use private API. The underscore at the beginning marks the function as private.
      (This has been the case for a long time; in 3.10 it is finally officially documented: https://docs.python.org/3.10/c-api/stable.html#stable )

      It has been removed in alpha5; I'm afraid rc2 is too late to put it back. (Especially since we'd need to put it back as public API, as CPython doesn't need it any more.)

      Please, if you see the need for any other private APIs, could you file bugs to make them public (or better, to support the use case they're needed for)? If they are useful, they should have documentation, tests, and better backwards compatibility expecations.

    8. serhiy-storchaka commented on Sep 27, 2021

      @serhiy-storchaka
      serhiy-storchaka
      on Sep 27, 2021
      MemberAuthor

      _PyImport_FindExtensionObject is a private API. It was added in 3.3 because import.c and importdl.c needed to share code. Since 3.5 it was only used in import.c, so there is no longer need to expose it. It was removed in 3.10 because there was an issue with this API: it returned a borroved reference which could be invalid at the time of returning.

      I can restore and fix _PyImport_FindExtensionObject in 3.10, but is not it too later to do after 3.10.0rc2?

      If it is essential to you, propose to add a new public API (perhaps better designed).

    9. indygreg commented on Sep 28, 2021

      @indygreg
      indygregmannequin
      on Sep 28, 2021
      Mannequin

      I didn't want to derail this old issue too much. So I filed bpo-45307 to track a solution.

    10. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    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

      3.10 (EOL)end of life3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions