Skip to content

Conversation

hartwork
Copy link
Contributor

@hartwork hartwork commented Sep 28, 2025

Within libexpat, a parser created via XML_ExternalEntityParserCreate
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser
to be garbage-collected by CPython. This fixes related reference
counting, to stop that from happening.
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, this looks like Tcl/Tk and their callbacks. If you want to check for reference leaks, you should also run the tests with -R (python -m test -R the_test)

@hartwork
Copy link
Contributor Author

Now, this looks like Tcl/Tk and their callbacks.

@picnixz I'm not sure what that means 😃

If you want to check for reference leaks, you should also run the tests with -R (python -m test -R the_test)

I cannot figure out why or what to do with python -m test -R the_test. Could you spell it out for me? 🙏

@hartwork hartwork requested a review from picnixz September 28, 2025 23:13
@picnixz
Copy link
Member

picnixz commented Sep 28, 2025

I cannot figure out why or what to do with python -m test -R the_test. Could you spell it out for me? 🙏

Sorry, to be precise the -R : option checks if there are reference leaks. In your case run:

$ ./python -m test test_pyexpat -m test_parent_parser_outlives_its_subparsers -R :

@picnixz
Copy link
Member

picnixz commented Sep 28, 2025

I'm not sure what that means

Tcl/Tk is a graphic library that allows to have delayed callbacks and the callback data is the Python object. Here, for Expat, the handler data is the XML parser itself as well. I wanted to make a parallel between those two constructions.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit tired (it's 01:33 AM here) so the review might not be perfect. I'll have another look tomorrow.

@hartwork
Copy link
Contributor Author

I cannot figure out why or what to do with python -m test -R the_test. Could you spell it out for me? 🙏

Sorry, to be precise the -R : option checks if there are reference leaks. In your case run:

$ ./python -m test test_pyexpat -m test_parent_parser_outlives_its_subparsers -R :

@picnixz after a recompile with --with-pydebug that worked — thank you! 🙏

@hartwork
Copy link
Contributor Author

I'm a bit tired (it's 01:33 AM here) so the review might not be perfect. I'll have another look tomorrow.

@picnixz no worries, same UTC+2 here. Thank you! 👍 👍

@hartwork
Copy link
Contributor Author

Related: libexpat/libexpat#1066

@hartwork hartwork requested a review from picnixz September 29, 2025 14:06
As suggested by @picnixz
@miss-islington-app
Copy link

Sorry, @hartwork and @picnixz, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6edb2ddb5f3695cf4938979d645f31d7fba43ec8 3.11

@miss-islington-app
Copy link

Sorry, @hartwork and @picnixz, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6edb2ddb5f3695cf4938979d645f31d7fba43ec8 3.13

@miss-islington-app
Copy link

Sorry, @hartwork and @picnixz, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6edb2ddb5f3695cf4938979d645f31d7fba43ec8 3.12

@picnixz
Copy link
Member

picnixz commented Oct 5, 2025

Lucky me, 3.14 has a clean backport :)

@picnixz
Copy link
Member

picnixz commented Oct 5, 2025

@hartwork Could you make the backports for 3.10-3.13 please? I'm not on my dev session now (sorry for bothering you with this).

@hartwork
Copy link
Contributor Author

hartwork commented Oct 5, 2025

@picnixz thanks for your help getting this over the finish line 🙏 🎉 I can look into the backports for this and report back if I hit any roadblocks.

@picnixz
Copy link
Member

picnixz commented Oct 5, 2025

Thank you for writing libexpat :')

hartwork added a commit to hartwork/cpython that referenced this pull request Oct 5, 2025
…rs in `pyexpat` (python#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139608 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 5, 2025
hugovk pushed a commit that referenced this pull request Oct 5, 2025
hartwork added a commit to hartwork/cpython that referenced this pull request Oct 5, 2025
…rs in `pyexpat` (python#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139609 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 5, 2025
hartwork added a commit to hartwork/cpython that referenced this pull request Oct 5, 2025
…rs in `pyexpat` (python#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139612 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 5, 2025
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139608 is a backport of this pull request to the 3.13 branch.

@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139609 is a backport of this pull request to the 3.12 branch.

@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139612 is a backport of this pull request to the 3.11 branch.

hartwork added a commit to hartwork/cpython that referenced this pull request Oct 5, 2025
…rs in `pyexpat` (python#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139613 is a backport of this pull request to the 3.10 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.10 only security fixes label Oct 5, 2025
hartwork added a commit to hartwork/cpython that referenced this pull request Oct 5, 2025
…rs in `pyexpat` (python#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2025

GH-139614 is a backport of this pull request to the 3.9 branch.

encukou pushed a commit that referenced this pull request Oct 6, 2025
…ers in `pyexpat` (GH-139403) (GH-139608)

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)
pablogsal pushed a commit that referenced this pull request Oct 6, 2025
…ers in `pyexpat` (GH-139403) (#139612)

* gh-139400: Make sure that parent parsers outlive their subparsers in `pyexpat` (#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)

* Move news item from section "Core and Builtins" to section "Security"
pablogsal pushed a commit that referenced this pull request Oct 6, 2025
…ers in `pyexpat` (GH-139403) (#139613)

* gh-139400: Make sure that parent parsers outlive their subparsers in `pyexpat` (#139403)

* Modules/pyexpat.c: Disallow collection of in-use parent parsers.

Within libexpat, a parser created via `XML_ExternalEntityParserCreate`
is relying on its parent parser throughout its entire lifetime.
Prior to this fix, is was possible for the parent parser to be
garbage-collected too early.

(cherry picked from commit 6edb2dd)

* Move news item to from section "Core and Builtins" to section "Security"
@bedevere-bot
Copy link

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

Hi! The buildbot AMD64 Fedora Stable Refleaks 3.11 (tier-1) has failed when building commit 1459d1f.

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/1027/builds/823) and take a look at the build logs.
  4. Check if the failure is related to this commit (1459d1f) 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/1027/builds/823

Failed tests:

  • test_socket

Failed subtests:

  • test_aead_aes_gcm - test.test_socket.LinuxKernelCryptoAPI.test_aead_aes_gcm

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

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-x86_64.refleak/build/Lib/test/test_socket.py", line 6557, in test_aead_aes_gcm
    op.sendall(plain)
OSError: [Errno 22] Invalid argument

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

Successfully merging this pull request may close these issues.

5 participants