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
AttributeError in Popen.__del__ #63221
Comments
Sometimes closing IDLE I got such exception: Exception ignored in: <bound method Popen.__del__ of <subprocess.Popen object at 0xb5b8618c>>
Traceback (most recent call last):
File "/home/serhiy/py/cpython/Lib/subprocess.py", line 890, in __del__
TypeError: 'NoneType' object is not callable It is happened because the builtins module was destroyed before running __del__ in process of garbage collecting. Here is a patch which should fix the bug. Anothe possible solution is get rid from getattr and catch AttributeError instead. See also bpo-16650. |
Surely this would suffer from the same issue? Why are the builtins getting deleted anyway? In fact, why is getattr getting deleted from the builtins module? The __builtins__ global is specifically not deleted when deleting modules to let destructors use builtins, and indeed the error message says "is not callable", suggesting the builtins module is still there but getattr is not. Is this perhaps some sort of IDLE bug? |
When the patch to bpo-12085 is changed, as has been agreed, I think, this issue should go away. Moving the deletion of builtins to later in the shutdown process has be discussed and maybe implemented. |
But the thing is, builtins are already supposed to be the very last thing destroyed at shutdown. |
I'm wondering too. While the code in Popen.__del__() is almost same, I can't reproduce the issue in 3.3. Perhaps it relates to some 3.4 changes? Import machinery, weak references, the shutdown process? Before applying the patch which fixes Popen.__del__() I want understand what happens. |
The clearing of modules at shutdown has been substantially changed in 3.4. Now a best effort is made to let the module go away purely by gc. Those modules which survive get purged in random order. In 3.3 all modules were purged, but builtins was special cased to be purged last. (See Python/import.c:PyImport_Cleanup().) I would favour setting a flag before the purging stage which prevents __del__ methods (and weakrefs?) from running. |
Could we instead restore the builtins module to it's initial state before the purging stage? I believe all builtins are immutable and can't create reference loops. |
I think your bpo-12085 patch should be applied and that issue closed. It is not specifically about shutdown. Tweaking shutdown further would be a new issue. |
There is a regression in 3.4 due to changes in shutdown procedure. This code correctly works in 3.3. There are more than a dozen places in the stdlib which rely upon accessibility of builtins. I wrote patches for all these cases, but third-party code will be broken. I think we should restore guarantees about builtins. |
Well, perhaps we can special-case builtins not to be "wiped" at shutdown. |
Maybe it is because it uses the evil resuscitate-in-del trick. I We could get rid of the trick:
The hardest thing about making such a change is that test_subprocess |
We're seeing this in Ubuntu now that 3.4 is the default. |
Serhiy fixed the issue bpo-19255 with the changeset 6a1711c96fa6, but this changeset will wait Python 3.4.1. |
On Feb 27, 2014, at 03:46 PM, STINNER Victor wrote:
Okay, thanks. I was reviewing and rather liked the less invasive patch, but |
If this fixes the problem, shouldn't the issue be closed and a NEWS item added? I'm going to test the patch locally. |
Hmm... if this *already* affects Ubuntu users, shouldn't this be fixed in 3.4 proper? It's extremely likely that someone else will be affected too. |
On Feb 27, 2014, at 07:23 PM, Antoine Pitrou wrote:
Agreed! |
I've testing this patch on Ubuntu, and it seems to fix the problem. My quick testing doesn't show any new problems, but we'll only know for sure once the new Python 3.4 package hits the archive and folks start updating to it. So far so good though. Larry, please consider cherry picking this into 3.4.0 final. |
Nosying Doko, since I think he may want to get this fix into Debian, if Larry does not cherry pick it into 3.4.0 final. |
Note that the changeset 6a1711c96fa6 introdused several errors (mostly tests failures), so it needs other changesets to fix it. Related changesets which should be cherry picked: 6a1711c96fa6, fa160c8145e5, efaf12106d68, 7ecee9e0dc58, 10ea3125d7b8, 488ccbee6ee6. |
On Feb 28, 2014, at 09:42 PM, Serhiy Storchaka wrote:
Interestingly enough, the test failures don't seem to break the build, but do Thanks for listing the additional revisions that need to be cherry picked. I |
So you're asking that I cherry pick six revisions here? 6a1711c96fa6 |
Those six revisions have been cherry-picked into 3.4.0. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: