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

gh-128923: Use zero to indicate unassigned unique id #128925

Merged
merged 6 commits into from
Jan 17, 2025

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Jan 16, 2025

In the free threading build, the per thread reference counting uses a unique id for some objects to index into the local reference count table. Use 0 instead of -1 to indicate that the id is not assigned. This avoids bugs where zero-initialized heap type objects look like they have a unique id assigned.

In the free threading build, the per thread reference counting uses a
unique id for some objects to index into the local reference count
table. Use 0 instead of -1 to indicate that the id is not assigned. This
avoids bugs where zero-initialized heap type objects look like they have
a unique id assigned.
Include/internal/pycore_object.h Show resolved Hide resolved
Modules/_testcapimodule.c Outdated Show resolved Hide resolved
Objects/codeobject.c Outdated Show resolved Hide resolved
Python/uniqueid.c Outdated Show resolved Hide resolved
@colesbury colesbury requested a review from Yhg1s January 17, 2025 15:19
@Yhg1s Yhg1s merged commit d66c08a into python:main Jan 17, 2025
39 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 Android 3.x has failed when building commit d66c08a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1594/builds/1046) and take a look at the build logs.
  4. Check if the failure is related to this commit (d66c08a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1594/builds/1046

Failed tests:

  • test.test_pydoc.test_pydoc

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_pydoc/test_pydoc.py", line 559, in test_builtin_with_more_than_four_children
    text = doc.docclass(object)
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pydoc.py", line 1436, in docclass
    subclasses = sorted(
        (str(cls.__name__) for cls in type.__subclasses__(object)
         if not cls.__name__.startswith("_") and cls.__module__ == "builtins"),
        key=str.lower
    )
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pydoc.py", line 1438, in <genexpr>
    if not cls.__name__.startswith("_") and cls.__module__ == "builtins"),
                                            ^^^^^^^^^^^^^^
AttributeError: __module__. Did you mean: '__reduce__'?

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x has failed when building commit d66c08a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/2346) and take a look at the build logs.
  4. Check if the failure is related to this commit (d66c08a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/2346

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Android 3.x has failed when building commit d66c08a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1591/builds/937) and take a look at the build logs.
  4. Check if the failure is related to this commit (d66c08a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1591/builds/937

Failed tests:

  • test.test_pydoc.test_pydoc

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_pydoc/test_pydoc.py", line 559, in test_builtin_with_more_than_four_children
    text = doc.docclass(object)
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pydoc.py", line 1436, in docclass
    subclasses = sorted(
        (str(cls.__name__) for cls in type.__subclasses__(object)
         if not cls.__name__.startswith("_") and cls.__module__ == "builtins"),
        key=str.lower
    )
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pydoc.py", line 1438, in <genexpr>
    if not cls.__name__.startswith("_") and cls.__module__ == "builtins"),
                                            ^^^^^^^^^^^^^^
AttributeError: __module__. Did you mean: '__reduce__'?

@Yhg1s
Copy link
Member

Yhg1s commented Jan 17, 2025

Re: the buildbot failures, they're happening because the newly added heaptype inherits from object and does not have the __module__ attribute. Given that creating classes this way (and them not having __module__) isn't inherently wrong, I think the correct fix is to make pydoc robust against this.

@Yhg1s
Copy link
Member

Yhg1s commented Jan 17, 2025

#128951 should fix the buildbot failures. (This wasn't picked up by GHA because we apparently have no GHA that runs the tests in a single process/sequentially.)

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

Successfully merging this pull request may close these issues.

4 participants