msg243893 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 07:53 |
Here is the implementation for the recently accepted PEP 489.
Tested on Linux.
|
msg243894 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 07:58 |
And here are all changes in a single patch.
|
msg243899 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 08:28 |
I'll get this merged tonight so we make the beta1 deadline, but I expect the initial docs to be fairly rudimentary and requiring further updates.
|
msg243910 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 12:29 |
Attached patch is the one I'm looking to commit, but -R 3:3 shows significant reference leaks in test_importlib, and possible problems in other tests as well.
I'm about to revert it to see if there were any pre-existing refleak issues before applying this.
|
msg243911 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 13:08 |
Initial implementation checked in at https://hg.python.org/cpython/rev/e729b946cc03
Larry, FYI regarding the refleak in test_importlib I just committed: as described in the commit message, I'm pretty sure it's a real refleak in the current PEP 489 implementation, it's just a beast to track down because there are a lot of moving parts when it comes to module cleanup, especially for extension modules.
Aside from that, the tests all pass, so I hope you're OK with my checking it in its current state for beta 1, with the aim of fixing the bug for beta 2 (I would have asked first, but time zones intervened - I'm about to head to bed, and I expect you'll have closed 3.5 to new features by the time I get up)
|
msg243913 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 13:12 |
Regarding the extraneous whitespace changes in modsupport.h, those are courtesy of running "make patchcheck" as part of preparing the commit.
|
msg243916 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 14:49 |
Fix some refleaks
- one in PyModule_FromDefAndSpec2 - this is my fault
- one in PyType_FromSpecWithBases - I guess this is
the first time PEP-384 built-in types are GC'd
- one in the new PEP 489 tests
There's still one more in the new testing module, _testmultiphase. I'm working on it.
|
msg243917 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-23 15:03 |
New changeset 7f2e6f236202 by Nick Coghlan in branch 'default':
Issue #24268: Address some PEP 489 refleaks
https://hg.python.org/cpython/rev/7f2e6f236202
|
msg243918 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 15:07 |
Thanks, that reduces the refleaks shown for "./python -m test -R3:3 test_importlib" from 102 to 50 for me.
However, I suspect there may still be a leak in the machinery, as I'm still seeing the total number of objects growing when importing the array module and then dropping the references to it:
$ ./python -X showrefcount
Python 3.5.0a4+ (default:e729b946cc03+, May 24 2015, 00:58:18)
[GCC 5.1.1 20150422 (Red Hat 5.1.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
[54384 refs, 15894 blocks]
>>> import array; del array; del sys.modules['array']
[54654 refs, 15974 blocks]
>>> import array; del array; del sys.modules['array']
[54695 refs, 15986 blocks]
>>> import array; del array; del sys.modules['array']
[54736 refs, 15997 blocks]
>>> import array; del array; del sys.modules['array']
[54777 refs, 16008 blocks]
>>> import array; del array; del sys.modules['array']
[54818 refs, 16019 blocks]
>>> import array; del array; del sys.modules['array']
[54859 refs, 16030 blocks]
|
msg243919 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-23 15:12 |
Once we sort these out, any that impact Python 3.4 (like the PyType_FromSpecAndBases one) should be backported to the maintenance branch.
I wouldn't assume the one you found in PyType_FromSpecAndBases is the only one, though - I believe the tests for this PEP are managing to exercise parts of the module cleanup machinery that it's never been practical to properly test in the past.
And now I really am heading to bed :)
|
msg243920 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 15:20 |
The array module is good if you *really* drop references:
$ ./python -X showrefcount
Python 3.5.0a4+ (default, May 23 2015, 16:44:38)
[GCC 4.9.2 20150212 (Red Hat 4.9.2-6)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys, gc
[54618 refs, 15960 blocks]
>>> import array; del array; del sys.modules['array']; gc.collect()
6
[54851 refs, 15973 blocks]
>>> import array; del array; del sys.modules['array']; gc.collect()
6
[54851 refs, 15973 blocks]
>>> import array; del array; del sys.modules['array']; gc.collect()
6
[54851 refs, 15973 blocks]
There is a cycle between a each built-in function and its module. Modules aren't optimized for being unloaded.
|
msg243921 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 15:24 |
FWIW, the remaining refleak occurs when unloading an extension module object. This is something that wasn't possible before PEP 489 -- extension modules were never deleted.
|
msg243927 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-23 16:00 |
New changeset a811f5561c99 by Steve Dower in branch 'default':
Issue #24268: Fixes generation of init import name on Windows.
https://hg.python.org/cpython/rev/a811f5561c99
|
msg243928 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-23 16:01 |
For the sake of getting things running again, I went ahead and fixed the format string in dynload_win.c. Feel free to change it again if that isn't what was intended.
|
msg243938 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 19:35 |
Thank you, Steve.
A similar problem is on other platforms as well. This patch should fix it; could someone look at it?
|
msg243941 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-23 20:17 |
That patch looks good to me. Totally didn't think to look for copy-paste issues...
|
msg243942 - (view) |
Author: Larry Hastings (larry) * |
Date: 2015-05-23 20:20 |
If it makes sense, can you guys check it in soon, like in real-time here? I tag 3.5 beta 1 in about an hour, and since this is a "bug-fix" it's legitimate to go in.
|
msg243943 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-23 21:01 |
Steve, could you please merge it?
|
msg243947 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-23 21:14 |
New changeset 7b5f5f8b26a6 by Steve Dower in branch 'default':
Issue #24268: Fix import naming when loading extension modules. Patch by Petr Viktorin.
https://hg.python.org/cpython/rev/7b5f5f8b26a6
|
msg243948 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2015-05-23 21:14 |
error C2079: 'PyModuleDef_Type' uses undefined struct '_typeobject' c:\cpython\include\moduleobject.h is occurring on both 32 and 64 bit release builds on Windows 8.1 VS2015. I don't know what is causing it but figured I'd better flag it up pronto, and as you guys are here...
|
msg243949 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-23 21:27 |
I think it's missing PyAPI_DATA, as it's probably supposed to be an external reference than a declaration. If it builds fine now, I'll commit that, but whoever made the change should confirm that's what it is supposed to be.
|
msg243950 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-23 21:37 |
It also required updating PC/python3.def to put the new functions into the stable API.
This is the reason we should auto-gen that file (+Zach), to make sure that anything people can #include under the limited ABI can actually be used.
Just double checking all the build configurations and then I'll push.
|
msg243951 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-23 21:44 |
New changeset af167a62e2a3 by Steve Dower in branch 'default':
Issue #24268: Adds PyModuleDef_Init and PyModuleDef_Type to python3.def (stable ABI)
https://hg.python.org/cpython/rev/af167a62e2a3
|
msg243953 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2015-05-23 22:31 |
FTR I've now built everything successfully.
|
msg243954 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-24 00:17 |
Thanks for getting the Windows side sorted out folks, and my apologies for
the breakage. The overlap between the current import system maintainers and
Windows developers is unfortunately the null set :(
|
msg243955 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-24 00:38 |
Since you're up, any chance you can help diagnose these test failures:
Traceback (most recent call last):
File "D:\buildarea\3.x.bolen-windows8\build\lib\test\test_importlib\extension\test_loader.py", line 93, in setUp
assert self.spec
AssertionError
From http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x/builds/925/steps/test/logs/stdio
It happens for 22 tests in test.test_importlib.extension.test_loader.Frozen_MultiPhaseExtensionModuleTests
|
msg243956 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-24 00:39 |
Actually, that probably means we're not building a new extension module on Windows, right?
|
msg243957 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-24 00:52 |
New changeset 42ec976f627e by Steve Dower in branch 'default':
Issue #24268: Adds PCBuild project to build _testmultiphase module.
https://hg.python.org/cpython/rev/42ec976f627e
|
msg243958 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-05-24 01:01 |
Confirmed that when we build the module needed by the test, the test works fine :)
Unfortunately, I think it missed the branch for b1, so people who install and run the test suite will see failures there. I'll make sure it gets into the 3.5 branch once that opens up.
|
msg244008 - (view) |
Author: Matthias Bussonnier (mbussonn) * |
Date: 2015-05-24 23:36 |
Hi,
Since the last few patches related to this, I seem to have an issue with `Python/importdl.c:get_encoded_name` (I guess) sometime returning the name with a leading dot. This lead to `PyInit_.modulename` being searched which fails.
My C-foo is extremely low, but changing [1] to `lastdot+1`
seem to do the trick for me (hence above supposition).
More especially in my case compiling Cython `failed to import Cython: dynamic module does not define module export function (PyInit_.Scanning)`, from `import Cython.Compiler.Scanning` I suppose. While it seem to does that ok with the `+1`.
I haven't found any related issues, and my read of pep 489 made me think this was not meant to change, Though I doubt my fix is correct.
Sorry if duplicate or already fixed, I searched as I could but did not found anything.
Thanks.
[1] https://hg.python.org/cpython/rev/e729b946cc03#l29.59
|
msg244029 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-25 11:45 |
Yes, you did find an error. Thanks for reporting it!
Here is a fix with a test case.
|
msg244034 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2015-05-25 13:37 |
See issue 24285, I ran into the same issue as mention in msg244008. That issue has a patch.
|
msg244038 - (view) |
Author: Matthias Bussonnier (mbussonn) * |
Date: 2015-05-25 14:45 |
> Yes, you did find an error. Thanks for reporting it!
> Here is a fix with a test case.
Thanks, I was unsure if there would have been side effect or other things to fix. I would have submitted a patch otherwise.
Thanks.
|
msg244100 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-05-26 12:00 |
I registered the fix for importing extension modules from packages against issue 24285 in the commit and NEWS file rather than against this original implementation RFE.
Commit references in http://bugs.python.org/issue24285#msg244098
|
msg244378 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2015-05-29 13:57 |
I'm seeing crashes with this assertion in Cython test modules:
python: ./Python/importdl.c:75: get_encoded_name: Assertion `(((PyObject*)(encoded))->ob_refcnt) == 1' failed.
The problem seems to be that the module name is only one character long ("a"), and single character byte strings are interned.
|
msg244382 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-29 14:46 |
Ah, indeed. I need to create a new bytes object here after all.
Here is a fix, with a test.
Thank you Stefan!
|
msg244383 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2015-05-29 15:04 |
Patch LGTM and it fixes the problem (tried it on my side). Please make sure it gets applied in time for beta 2.
|
msg244404 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-29 19:51 |
I've opened issue24328 for that regression.
|
msg244405 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-05-29 19:57 |
And, the remaining refleak is a known issue from 2012: PyType_FromSpec-created types with custom tp_dealloc leak references when instantiated.
See issue 16690
There's more discussion is in issue 15653
Martin v. Löwis notes:
> So I'd propose that it is actually the leaf subtype which decrefs
> ob_type. The check whether you are the leaf type is then done by
> checking whether tp_dealloc is the one you are "in" right now.
Is that really the way to go? I didn't find this info in the docs, should those be updated?
|
msg244790 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2015-06-03 22:31 |
I've posted a patch that fixes the remaining refleak in issue24373.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:17 | admin | set | github: 68456 |
2015-06-03 22:31:45 | petr.viktorin | set | messages:
+ msg244790 |
2015-05-29 19:57:23 | petr.viktorin | set | messages:
+ msg244405 |
2015-05-29 19:51:11 | petr.viktorin | set | messages:
+ msg244404 |
2015-05-29 15:04:26 | scoder | set | messages:
+ msg244383 |
2015-05-29 14:46:57 | petr.viktorin | set | files:
+ fix-short-names.patch
messages:
+ msg244382 |
2015-05-29 13:57:04 | scoder | set | nosy:
+ scoder messages:
+ msg244378
|
2015-05-28 14:33:32 | brett.cannon | set | nosy:
+ brett.cannon
|
2015-05-26 12:00:28 | ncoghlan | set | status: open -> closed type: enhancement messages:
+ msg244100
resolution: fixed stage: resolved |
2015-05-25 21:22:20 | berker.peksag | link | issue24285 superseder |
2015-05-25 14:45:50 | mbussonn | set | messages:
+ msg244038 |
2015-05-25 13:37:53 | ronaldoussoren | set | nosy:
+ ronaldoussoren messages:
+ msg244034
|
2015-05-25 11:45:41 | petr.viktorin | set | files:
+ fix-pep489-submodule.patch
messages:
+ msg244029 |
2015-05-24 23:36:15 | mbussonn | set | nosy:
+ mbussonn messages:
+ msg244008
|
2015-05-24 01:01:39 | steve.dower | set | messages:
+ msg243958 |
2015-05-24 00:52:32 | python-dev | set | messages:
+ msg243957 |
2015-05-24 00:39:52 | steve.dower | set | messages:
+ msg243956 |
2015-05-24 00:38:25 | steve.dower | set | messages:
+ msg243955 |
2015-05-24 00:17:47 | ncoghlan | set | messages:
+ msg243954 |
2015-05-23 22:31:07 | BreamoreBoy | set | messages:
+ msg243953 |
2015-05-23 21:44:50 | python-dev | set | messages:
+ msg243951 |
2015-05-23 21:37:00 | steve.dower | set | nosy:
+ zach.ware messages:
+ msg243950
|
2015-05-23 21:27:35 | steve.dower | set | messages:
+ msg243949 |
2015-05-23 21:14:48 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg243948
|
2015-05-23 21:14:19 | python-dev | set | messages:
+ msg243947 |
2015-05-23 21:01:18 | petr.viktorin | set | messages:
+ msg243943 |
2015-05-23 20:20:10 | larry | set | messages:
+ msg243942 |
2015-05-23 20:17:47 | steve.dower | set | messages:
+ msg243941 |
2015-05-23 19:35:27 | petr.viktorin | set | files:
+ fix-dynload-init-name.patch
messages:
+ msg243938 |
2015-05-23 16:01:14 | steve.dower | set | nosy:
+ steve.dower messages:
+ msg243928
|
2015-05-23 16:00:06 | python-dev | set | messages:
+ msg243927 |
2015-05-23 15:24:27 | petr.viktorin | set | messages:
+ msg243921 |
2015-05-23 15:20:10 | petr.viktorin | set | messages:
+ msg243920 |
2015-05-23 15:12:16 | ncoghlan | set | messages:
+ msg243919 |
2015-05-23 15:07:44 | ncoghlan | set | messages:
+ msg243918 |
2015-05-23 15:03:59 | python-dev | set | nosy:
+ python-dev messages:
+ msg243917
|
2015-05-23 14:49:40 | petr.viktorin | set | files:
+ refleak-fix.patch
messages:
+ msg243916 |
2015-05-23 13:12:10 | ncoghlan | set | messages:
+ msg243913 |
2015-05-23 13:08:38 | ncoghlan | set | nosy:
+ larry messages:
+ msg243911
|
2015-05-23 12:29:59 | ncoghlan | set | files:
+ pep0489-multiphase-extension-module-import-v2.patch
messages:
+ msg243910 |
2015-05-23 08:28:44 | ncoghlan | set | assignee: ncoghlan messages:
+ msg243899 versions:
+ Python 3.5 |
2015-05-23 07:58:06 | petr.viktorin | set | files:
+ pep0489.patch keywords:
+ patch messages:
+ msg243894
|
2015-05-23 07:53:42 | petr.viktorin | create | |