Support PURE_PYTHON instead of AC_PURE_PYTHON #120
Conversation
…PYTHON. I disabled the tests which explicitly test the C implementation so only the actually problematic ones remain.
|
Interestingly GHA fails in other interesting ways: https://github.com/zopefoundation/AccessControl/runs/4350600029?check_suite_focus=true |
|
Michael Howitz wrote at 2021-11-28 23:43 -0800:
Interestingly GHA fails in other interesting ways: https://github.com/zopefoundation/AccessControl/runs/4350600029?check_suite_focus=true
In the "PURE_PYTHON" version, `AccessControl` should not import
`cAccessControl` as it does in
File "/home/runner/work/AccessControl/AccessControl/src/AccessControl/ImplPython.py", line 33, in <module>
from AccessControl.cAccessControl import _what_not_even_god_should_do
Instead, it should define `_what_not_even_god_should_do` itself.
The likely problem is that `cAccessControl` internally uses the
C implementation of `ExtensionClass` but the expectations of that
are not compatible with `PURE_PYTHON`.
|
I did this in 0005e9f – locally it did not change anything. The failures shown in the description of this PR stay the same. |
|
https://github.com/zopefoundation/AccessControl/runs/4364181714?check_suite_focus=true now shows the same failures I see locally using |
This is likely a limitation of the Python implementation for |
|
@d-maurer Thank you for the explanation. What do you suggest as next steps? |
|
Michael Howitz wrote at 2021-12-1 23:19 -0800:
***@***.*** Thank you for the explanation. What do you suggest as next steps?
I am working on this.
I have already 2 ideas to resolve the `Acquisition` problem.
The failing refcount test looks strange -- as if something would
create cycles. I will investigate once I can reproduce the
problem in my typical environment (no `tox`).
|
Indeed: something seems to have created cycles involving (Pdb) !from gc import get_referrers
(Pdb) !gr = get_referrers
(Pdb) pp gr(eo)
[[<AccessControl.tests.testZopeSecurityPolicy.ImplictAcqObject object at 0x7f981dc28470>],
<frame object at 0x7f981dc293e8>,
<frame object at 0x178e9f8>,
<frame object at 0x17f6378>,
<frame object at 0x1826408>,
<frame object at 0x1827138>,
<frame object at 0x176f938>,
<frame object at 0x1826e78>,
<frame object at 0x182b908>,
<frame object at 0x182b138>,
<frame object at 0x182bc58>,
{'eo': <AccessControl.tests.testZopeSecurityPolicy.ImplictAcqObject object at 0x7f981dc28470>,
'get_referrers': <built-in function get_referrers>,
'gr': <built-in function get_referrers>,
'rc': 3,
'self': <AccessControl.tests.testZopeSecurityPolicy.Python_ZSPTests testMethod=testIdentityProxy>}]
(Pdb) !import gc; gc.collect()
188
(Pdb) p sys.getrefcount(eo)
4
(Pdb) pp gr(eo)
[{'eo': <AccessControl.tests.testZopeSecurityPolicy.ImplictAcqObject object at 0x7f981dc28470>,
'gc': <module 'gc' (built-in)>,
'get_referrers': <built-in function get_referrers>,
'gr': <built-in function get_referrers>,
'rc': 3,
'self': <AccessControl.tests.testZopeSecurityPolicy.Python_ZSPTests testMethod=testIdentityProxy>},
[<AccessControl.tests.testZopeSecurityPolicy.ImplictAcqObject object at 0x7f981dc28470>],
<frame object at 0x7f981dc293e8>]I push a commit which calls |
I tried to locate the cycles -- but I failed. The likely reason is that When I remember right, then With the changes in this PR and zopefoundation/Acquisition#57 the tests should pass. I will merge the I suggest again, that we harmonize |
…are not avoided in `Acquisition`
I have found the cause of the reference cycles --> "zopefoundation/Acquisition#57 (comment)". |
|
Now only Python 3.8 on Ubuntu fails reliably, see https://github.com/zopefoundation/AccessControl/runs/4557545726?check_suite_focus=true @d-maurer Do you have any idea? |
The test failure indicates that there are fewer references than expected. The huge number of references (in the 30,000) indicates that the corresponding object is extremely popular -- and therefore quite unfit to be used in a reference count test. I assume that the test failure results from an intervening garbage collection (when garbage collection is activated can depend on small variations, e.g. the Python version). In my view, the test is buggy. It should use a new object, not one referenced from thousands of places. If we do not want to change the test object, we can call the garbage collector manually before the |
|
@d-maurer Thank you for looking again into this issue. I saw that another test disables |
|
Tests were successfully running at https://github.com/zopefoundation/AccessControl/actions/runs/1828061585. (Had to start them manually as GH had disabled the workflow.) So I think we can prepare to merge this PR. |
icemac
changed the title
icemac
changed the title
icemac
changed the title
icemac
changed the title
I fixed the obvious problems and disabled the tests which explicitly test the C implementation so only the actually problematic ones remain.
Locally I get the following result for these changes:
See #119 for the base of this PR.