[issue46657] Add mimalloc memory allocator
Neil Schemenauer added the comment: Thanks, I'm indeed interested. Most credit goes to Christian for advancing this. For the missing stdatomic.h, would it be appropriate to have an autoconfig check for it? Can just disable mimalloc if it doesn't exist. -- ___ Python tracker <https://bugs.python.org/issue46657> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46657] Add mimalloc memory allocator
Neil Schemenauer added the comment: My preference would be for --with-mimalloc=yes in an upcoming release. For platforms without the required stdatomic.h stuff, they can manually specify --with-mimalloc=no. That will make them aware that a future release of Python might no longer build (if mimalloc is no longer optional). A soft-landing for merging nogil is not a good enough reason to merge mimalloc, IMHO. nogil may never be merged. There should be some concrete and immediate advantage to switch to mimalloc. The idea of using the "heap walking" to improve is cyclic GC is not concrete enough. It's just an idea at this point. I think the (small) performance win could be enough of a reason to merge. This seems to be the most recent benchmark: https://gist.github.com/pablogsal/8027937b71cd30f175ef7c885d3e There is also the long-term maintenance issue. So far, mimalloc upstream has been responsive. The mimalloc code is not so huge or complicated that we couldn't maintain it (if for some reason it gets abandoned upstream). However, I think we would prefer to maintain obmalloc rather than mimalloc, all else being equal. Abandonment by the upstream seems fairly unlikely. So, I'm not too concerned about maintenance. -- ___ Python tracker <https://bugs.python.org/issue46657> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1185] py3k: Completely remove nb_coerce slot
Neil Schemenauer added the comment: I made the changes as suggest by Guido. Commited as SVN rev 58226. Thanks for the patch. -- nosy: +nas __ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1185> __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1134] Parsing a simple script eats all of your memory
Neil Schemenauer added the comment: It looks to me like fp_readl is no longer working as designed and the patch is not really the right fix. The use of "decoding_buffer" is tricky and I think the conversion to bytes screwed it up. It might be clearer to have a separate "decoding_overflow" struct member that's used for overflow rather than overloading "decoding_buffer". -- nosy: +nas __ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1134> __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Neil Schemenauer added the comment: I'm late to the party but looks like Mark has a good handle on the issues. I would recommend not changing behavior in a bugfix release. I'm pretty sure code "in the wild" would be broken. -- ___ Python tracker <http://bugs.python.org/issue9011> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7759] mhlib fails on Btrfs filesystem (test_mhlib failure)
Neil Schemenauer added the comment: Closing this bug. I don't think it makes sense to change the mhlib module in bugfix release. My patch is fairly simple but not simple enough to make me feel comfortable. -- resolution: -> wont fix status: open -> closed ___ Python tracker <http://bugs.python.org/issue7759> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10241] gc fixes for module m_copy attribute
New submission from Neil Schemenauer : I'm trying implement some saner module shutdown procedure (similar to issue 812369). One of the many problems I've run into is that the GC doesn't know about the m_copy attribute of modules. I think the attached patch is correct. tp_tranverse exposes m_copy if it is non-NULL. tp_clear calls Py_CLEAR on it. -- assignee: loewis components: Interpreter Core files: module_m_copy.txt messages: 119957 nosy: benjamin.peterson, loewis, nascheme priority: normal severity: normal status: open title: gc fixes for module m_copy attribute type: behavior Added file: http://bugs.python.org/file19423/module_m_copy.txt ___ Python tracker <http://bugs.python.org/issue10241> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10255] refleak in initstdio
New submission from Neil Schemenauer : It looks to me like initstdio leaks a reference to "open". AFAIK, PyObject_SetAttrString() does not steal a reference. -- assignee: pitrou components: Interpreter Core files: initstdio_refleak.txt messages: 120008 nosy: nascheme, pitrou priority: normal severity: normal status: open title: refleak in initstdio type: resource usage Added file: http://bugs.python.org/file19434/initstdio_refleak.txt ___ Python tracker <http://bugs.python.org/issue10255> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10241] gc fixes for module m_copy attribute
Neil Schemenauer added the comment: Oops, my patch doesn't work since m_base can be shared by more than one module instance. I guess a different solution would be to cleanup the m_copy references on interpreter shutdown. Somehow they would have to be found though. -- ___ Python tracker <http://bugs.python.org/issue10241> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10241] gc fixes for module m_copy attribute
Neil Schemenauer added the comment: The attached patch seems better. The copies of module dicts stored in the interpreter state are dereferenced when the interpreter shuts down. -- Added file: http://bugs.python.org/file19509/module_m_copy2.txt ___ Python tracker <http://bugs.python.org/issue10241> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10333] Remove ancient backwards compatibility GC API
New submission from Neil Schemenauer : I think it should be safe to remove some backwards compatibility cruft. This was introduced during the 2.3 development cycle. -- files: gc_cruft.txt messages: 120554 nosy: nascheme priority: low severity: normal status: open title: Remove ancient backwards compatibility GC API Added file: http://bugs.python.org/file19510/gc_cruft.txt ___ Python tracker <http://bugs.python.org/issue10333> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3054] test_disutils fails
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: Looks like this has exposed some ugly code. From setup.py: # Figure out the location of the source code for extension modules # (This logic is copied in distutils.test.test_sysconfig, # so building in a separate directory does not break test_distutils.) I believe the proper fix is to move some of the code from setup.py into sysconfig so that it does more than just set "python_build" when running inside a Python build directory. For example, it should also add /Include to the standard list of includes that distutils uses. The attached patch fixes test_build_ext.py to use the right source file (perhaps it should be tested on Windows). The test still fails because Python.h cannot be found. -- nosy: +nas Added file: http://bugs.python.org/file10659/test_ext_src.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue3054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3054] test_disutils fails
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: I think my previous patch combined with sysconfig_builddir.patch fixes this issue. Someone will need to test on other platforms. Note that the messy code in setup.py and in the tests should still be cleaned up. BTW, distutils is a den of stinking evil. ;-) Added file: http://bugs.python.org/file10660/sysconfig_builddir.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue3054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3054] test_disutils fails
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: One final patch for today (get_python_inc.patch). The patch combines my previous two patches and also cleans up some ugly code in setup.py and test_sysconfig.py. The source of the ugliness was that get_python_inc() did not work when running from within a build directory. The patch also fixes the header dependency feature introduced in svn r60287 (it assumed that builddir == srcdir and also that os.path.sep == '/'). Added file: http://bugs.python.org/file10663/get_python_inc.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue3054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3054] test_disutils fails
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file10659/test_ext_src.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue3054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue3054] test_disutils fails
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file10660/sysconfig_builddir.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue3054> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7753] newgil backport
Neil Schemenauer added the comment: Looks like the 2.7 patch has a spurious change to /Lib/unittest/runner.py. -- nosy: +nascheme ___ Python tracker <http://bugs.python.org/issue7753> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7753] newgil backport
Neil Schemenauer added the comment: Here is an updated set of patches that includes the ceval_gil.h file as well as the documentation changes. -- Added file: http://bugs.python.org/file15974/gil-2.7.txt ___ Python tracker <http://bugs.python.org/issue7753> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4698] Solaris buildbot failure on trunk in test_hostshot
Changes by Neil Schemenauer : -- resolution: -> fixed status: open -> closed ___ Python tracker <http://bugs.python.org/issue4698> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1220756] Re-importing embedded thread dumps core
Changes by Neil Schemenauer : -- resolution: -> fixed status: open -> closed ___ Python tracker <http://bugs.python.org/issue1220756> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7759] mhlib fails on Btrfs filesystem (test_mhlib failure)
New submission from Neil Schemenauer : Btrfs does not maintain a link count for directories (MacOS does the same I think). That confuses mhlib.py because it uses os.stat().st_nlinks as an optimization. The attached patch removes the optimization and make test_mhlib pass on Btrfs (and probably HFS+) filesystems. -- files: mhlib_nlinks.txt messages: 98169 nosy: nascheme priority: normal severity: normal status: open title: mhlib fails on Btrfs filesystem (test_mhlib failure) type: behavior versions: Python 2.7, Python 3.2 Added file: http://bugs.python.org/file15975/mhlib_nlinks.txt ___ Python tracker <http://bugs.python.org/issue7759> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7759] mhlib fails on Btrfs filesystem (test_mhlib failure)
Neil Schemenauer added the comment: On Sat, Jan 23, 2010 at 06:09:33PM +, Antoine Pitrou wrote: > The documentation mentions that mhlib is deprecated and mailbox > should be used instead. Is there any point in trying to fix it? It looks like Btrfs will eventually conform to traditional st_nlink behavior. However, that still leaves HFS+. Perhaps the easiest fix would be to have the unit test check for weird st_nlink behavior by creating a directory with a subdirectory. If something is weird, skip testing mhlib. The downside to that solution is that someone might use mhlib on a HFS+ filesystem and encounter buggy behavior. I can imagine that removing the optimization can make mhlib much slower for large mail boxes. Maybe that would be better than risking lost mail though. On modern machines maybe it doesn't matter much. Neil -- ___ Python tracker <http://bugs.python.org/issue7759> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7759] mhlib fails on Btrfs filesystem (test_mhlib failure)
Neil Schemenauer added the comment: On Sun, Jan 24, 2010 at 01:25:18AM +, Antoine Pitrou wrote: > That wasn't really my question. What I ask is: since mhlib is > deprecated, why do we need to fix it while people are encouraged to use > mailbox instead? Sorry, I don't understand what you are proposing. Do you mean we should just let the test fail for people who develop on HFS+ and Btrfs filesystems? That seems not so good. > And, besides, does mailbox show the same problem? No, it doesn't have that optimization. Neil -- ___ Python tracker <http://bugs.python.org/issue7759> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7881] Hardcoded path, unsafe tempfile in test_logging
New submission from Neil Schemenauer :
The commit for issue #7868 added the following line to test_logging:
print >> open('/tmp/tmp.txt', 'w'), type(logger)
I'm not sure if that was intentional but it should be fixed. For one, that
path does not necessarily exist. Secondly, opening a file in a world writable
directory like that is a potential security problem. A simple fix would be to
use tempfile.TemporaryFile().
--
assignee: vinay.sajip
components: Tests
messages: 99032
nosy: nascheme, vinay.sajip
severity: normal
stage: needs patch
status: open
title: Hardcoded path, unsafe tempfile in test_logging
type: behavior
versions: Python 2.7
___
Python tracker
<http://bugs.python.org/issue7881>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue7885] test_distutils fails if Python built in separate directory
New submission from Neil Schemenauer : Lib/test/test_distutils.py crashes if Python was built in a directory other than the source directory. Using a separate build directory is handy if you are building Python with different sets of configure options since you only need one copy of the source. For example, in the source directory: $ mkdir build-opt $ cd build-opt $ ../configure $ make $ cd .. $ mkdir build-debug $ cd build-debug $ ../configure --with-pydebug $ make I fixed a similar problem in rev 69304 but now it is broken again. I get: /tmp/tmpbxrmiB/xxmodule.c:17:20: error: Python.h: No such file or directory Probably it is looking in the wrong directory for Python.h (assuming that the cwd is the top-level directory of the source). -- assignee: tarek components: Tests messages: 99054 nosy: nascheme, tarek priority: normal severity: normal stage: needs patch status: open title: test_distutils fails if Python built in separate directory type: behavior versions: Python 2.7, Python 3.2 ___ Python tracker <http://bugs.python.org/issue7885> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1266570] PEP 349: allow str() to return unicode
Neil Schemenauer added the comment: On Sat, Dec 08, 2007 at 05:14:45AM -, Alexandre Vassalotti wrote: > The PEP has been deferred and the patch is out of date. So, is this > change still wanted? I still think it would make things easier for people using Unicode with Python 2.x. However, there does not seem to be much community interest in the change. Neil _ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1266570> _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue628842] Deprecate PyNumber_Check
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: -- status: open -> closed Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue628842> ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1251748] compiler package: "global a; a=5"
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: On Sun, Mar 23, 2008 at 04:17:12PM +, Antoine Pitrou wrote: > Armin, if you still care about the compiler package, could you (or some > other pypy coder) take a look at #2459? As part of the patch, it > rewrites the flow graph block ordering algorithm in a cleaner way (IMHO). I'm interested to update the compiler package but I'm short of time at the moment. If you want to assign bugs and patches to me, I will try to get to them. Neil _ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1251748> _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2250] rlcompleter raises Exception on bad input
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: On Thu, Mar 20, 2008 at 08:49:32PM +, Sean Reifschneider wrote: > Is a straightforward patch, but I'd like NAS to comment on the change in > behavior. Probably would also need a documentation change, are you up > for doing that Lorenz? I doubt that people are relying on complete() to raise NameError or Attribute error. I think it would be okay just to trap them and not change the docs. A note in NEWS should be good enough. Neil __ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue2250> __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2467] gc.DEBUG_STATS reports invalid "elapsed" times
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: The original code is pretty icky. I'm attaching a patch that improves it, IMO. Before the elapsed time was only shown if garbage was found. I think it should always be shown if DEBUG_STATS is set. -- nosy: +nas Added file: http://bugs.python.org/file9843/gc_timestat.patch __ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue2467> __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1251748] compiler package: "global a; a=5"
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: On Tue, Mar 25, 2008 at 09:18:38AM +, Armin Rigo wrote: > The situation is that by now PyPy has found many many more bugs in > trying to use the compiler package to run the whole stdlib and > real-world applications. What I can propose is to extract what we have > got and put it back into CPython's stdlib, keeping the current > documented API. I'm not sure it can go back into the stdlib since there doesn't seem to be enough energy available to keep it up-to-date with the Python release schedule. I would like to make it available as an add-on package. > This will require a little bit of work (e.g. first > finishing to add all new 2.5 and 2.6 features into PyPy's compiler) > but IMHO it's more worthwhile than going through the process of > rediscovering and fixing all the current bugs one by one. Indeed it would make no sense to redo that work. Can the version in the PyPy tree be used as a direct replacement for the stdlib version or does it need some changes? You had mentioned being able to produce a patch against the stdlib version. Is that easy or would it be better just to take the PyPy version and package it up. Neil _ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1251748> _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
New submission from Neil Schemenauer <[EMAIL PROTECTED]>: Building in a separate directory got broken at some point. The code is hairy but it looks like the source of the problem was a lame sysconfig.get_python_inc() function. The attached patches fix things and hopefully do not introduce any new bugs. -- components: Build files: get_python_inc.patch keywords: patch messages: 74996 nosy: nas severity: normal status: open title: Separate build dir broken type: compile error Added file: http://bugs.python.org/file11837/get_python_inc.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: Added file: http://bugs.python.org/file11838/get_python_inc2.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file11837/get_python_inc.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Changes by Neil Schemenauer <[EMAIL PROTECTED]>: Added file: http://bugs.python.org/file11839/get_python_inc2.patch ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4152] ihooks module cannot handle absolute imports
New submission from Neil Schemenauer <[EMAIL PROTECTED]>: The ihooks module was updated when the absolute imports feature was implemented. At a minimum, I guess the import_module() methods in that module would need to have "level=-1" keyword arguments added. The only library in the core that uses an absolute import is encodings/__init__.py. -- messages: 74998 nosy: nas severity: normal status: open title: ihooks module cannot handle absolute imports type: behavior versions: Python 2.6 ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4152> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4070] python tests failure if builddir <> sourcedir
Neil Schemenauer <[EMAIL PROTECTED]> added the comment: Issue 4151 contains a similar patch but it cleans up more code, IMHO. -- nosy: +nas ___ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue4070> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Neil Schemenauer <[EMAIL PROTECTED]> added the comment:
[Roumen Petrov]
> Modification for test_build_ext.py from above mentioned issue break
> non-posix builds.
Without that change the test fails on Unix platforms when the build is
not done inside the source directory. I guess the problem must be the
change from (essentially):
def _get_source_filename():
return os.path.join(sysconfig.project_base, 'Modules', 'xxmodule.c')
to
def _get_source_filename():
srcdir = sysconfig.get_config_var('srcdir')
return os.path.join(srcdir, 'Modules', 'xxmodule.c')
On POSIX, project_base is just the directory containing the Python
executable. I think the right thing to do is to fix the srcdir var on
non-POSIX systems. I don't have a Windows machine to test on. Could
someone test the get_python_inc2.patch with the non_posix_srcdir.patch
applied on top of it?
Added file: http://bugs.python.org/file11858/non_posix_srcdir.patch
___
Python tracker <[EMAIL PROTECTED]>
<http://bugs.python.org/issue4151>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Neil Schemenauer added the comment: I committed my proposed changes in several chunks, ending with r69305. I think building in a separate directory again works and that non-POSIX platforms are not adversely affected by this change. ___ Python tracker <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Neil Schemenauer added the comment: I committed my proposed changes in several chunks, ending with r69305. I think building in a separate directory again works and that non-POSIX platforms are not adversely affected by this change. -- nosy: +nascheme resolution: -> fixed status: open -> closed ___ Python tracker <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Changes by Neil Schemenauer : ___ Python tracker <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4070] python tests failure if builddir <> sourcedir
Changes by Neil Schemenauer : -- assignee: -> nascheme nosy: +nascheme resolution: -> fixed status: open -> closed ___ Python tracker <http://bugs.python.org/issue4070> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Neil Schemenauer added the comment: Roumen, thanks for reporting the bug. It seems that distutils requires an absolute path (although I didn't waste time digging too deep into the issue). ___ Python tracker <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5161] wrong paths for ctypes cleanup
New submission from Neil Schemenauer :
The following code was added to Makefile.pre.in when ctypes was merged:
find $(srcdir)/build -name 'fficonfig.h' -exec rm -f {} ';' || true
find $(srcdir)/build -name 'fficonfig.py' -exec rm -f {} ';' || true
If Python is compiled in a directory other than the source directory
than $(srcdir)/build does not exist and these commands don't work as
intended. I think that proper path would be simply "build".
--
assignee: theller
components: Build
messages: 81243
nosy: nascheme, theller
priority: low
severity: normal
status: open
title: wrong paths for ctypes cleanup
type: compile error
___
Python tracker
<http://bugs.python.org/issue5161>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2472] Fixed block ordering code in compiler.pyassem
Neil Schemenauer added the comment: Thanks Antoine. For some reason I don't think I ever got an email about this issue. I did some further cleanups and optimizations. Committed as SVN rev 69373. Lib/compiler is still in need of some fixing since it doesn't handle decorators and some other new language features. I'll try to keep at it. -- assignee: -> nascheme resolution: -> accepted stage: -> committed/rejected status: open -> closed ___ Python tracker <http://bugs.python.org/issue2472> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1544277] python compiler support for with stmt
Neil Schemenauer added the comment: Looks like this has already been fixed in SVN rev 53575. -- resolution: -> out of date stage: -> committed/rejected status: open -> closed ___ Python tracker <http://bugs.python.org/issue1544277> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1754094] Tighter co_stacksize computation in stackdepth_walk
Changes by Neil Schemenauer : -- assignee: -> nascheme nosy: +nascheme ___ Python tracker <http://bugs.python.org/issue1754094> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue999042] Compiler module doesn't handle global statement correctly
Neil Schemenauer added the comment: Fixed in SVN rev 69394 (finally). This was done by having the symbol table differentiate between explicit and implicit globals. -- assignee: jhylton -> nascheme nosy: +nascheme resolution: -> fixed status: open -> closed ___ Python tracker <http://bugs.python.org/issue999042> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue999444] compiler module doesn't support unicode characters in laiter
Changes by Neil Schemenauer : -- assignee: jhylton -> nascheme nosy: +nascheme ___ Python tracker <http://bugs.python.org/issue999444> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5064] compiler.parse raises SyntaxErrors without line number information
Changes by Neil Schemenauer : -- assignee: -> nascheme nosy: +nascheme ___ Python tracker <http://bugs.python.org/issue5064> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5178] Add content manager for temporary directory
New submission from Neil Schemenauer : I noticed that it would be nice to have a temporary directory context manager while trying to fix a broken unittest. The attached patch provides a pretty minimal implementation. There appears to be lots of unit tests that could use such a thing (just search for "rmtree"). I'm not sure if TemporaryDirectory is the best name. Also, perhaps it should have a __del__ method although that's tricky business. -- assignee: ncoghlan files: tempdir.patch keywords: patch messages: 81342 nosy: nascheme, ncoghlan severity: normal stage: patch review status: open title: Add content manager for temporary directory type: feature request Added file: http://bugs.python.org/file12970/tempdir.patch ___ Python tracker <http://bugs.python.org/issue5178> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4151] Separate build dir broken
Neil Schemenauer added the comment: On Thu, Feb 12, 2009 at 01:32:37PM +, Matthias Klose wrote: > still seen on the 2.6 branch. applying r69374 on the branch doesn't fix > it there. The fix is spread over a number of commits: r69374, r69322, r69315, r69305, r69304, r69303, r69302. It could be backported but changing the build system is always fraught with portability traps and that's why it ends up turning to crud over time. Obviously most core Python developers don't build in a separate directory otherwise this feature won't have been broken for so long. ___ Python tracker <http://bugs.python.org/issue4151> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5178] Add context manager for temporary directory
Neil Schemenauer added the comment: New version of the patch. Added a __del__ method that hopefully works reliably. Added NEWS and an example of TemporaryFile as a context manager. I did not change more tests to use TemporaryDirectory since I understand hit-and-run modernization of code is generally discouraged. That said, I think there are some tests that could have their reliability improved by using TemporaryDirectory (I started on this patch when I saw some build bot failures). I'll look into that later. Added file: http://bugs.python.org/file13080/tempdir2.patch ___ Python tracker <http://bugs.python.org/issue5178> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5178] Add context manager for temporary directory
Neil Schemenauer added the comment: Argh, it's like pulling a sweater thread. The stat.S_ISDIR function uses the S_IFDIR global so I would have write my own. This does not look like good maintainable code, IMO. Maybe there should be a special flag modules can set to make them get cleared later in the interpreter cleanup procedure. Modules could set it if they don't create reference cycles themselves and don't hold references to user objects. ___ Python tracker <http://bugs.python.org/issue5178> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue812369] module shutdown procedure based on GC
Neil Schemenauer added the comment: This sounds like a nice idea. The current cleanup procedure in pythonrun.c is pretty lame since it can play havoc with __del__ methods (e.g. if they run after globals have been cleared). I've updated the patch to work with the current SVN head. Probably this should get tested with big applications based on Zope, Django, etc. -- nosy: +nascheme Added file: http://bugs.python.org/file13091/0001-update-GC-shutdown-patch.patch ___ Python tracker <http://bugs.python.org/issue812369> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: A few comments from the "peanut gallery". Thanks to Victor and Pablo for doing the hard work of investigating this bug. First, it has been a long time since Tim and I first developed gcmodule.c. So, some of my understanding may be inaccurate due to code changes or old age memory loss. ;-P If I understand Victor's test case correctly, the problem is caused if you have an extension type that implements tp_traverse but not tp_clear and that there is also a weakref involved in the trash cycle. In my original design of the GC, not implementing tp_clear should have been okay. It would mean that the GC might find garbage reference cycles that it couldn't cleanup. Those would leak memory but would not crash Python. I'm not sure what has changed since to require that tp_clear actually successfully clears the cycle. Tim was the origin of the code that handles weakrefs. The GC is (relatively) simple if not for handling weakrefs and finalizers. Tim did almost all of the difficult and brain exploding work on that stuff. There was a big re-org some years ago to handle "legacy finalizers" and PEP 442 finalizers. That made things more complicated yet. Maybe the requirement for a working tp_clear came into existence then? I added Tim to the nosy list since he might have insight. To me, it seems problematic that we would require a type to have a tp_clear method that actually breaks cycles. For mutable objects like dicts and lists, we can have tp_clear do its thing while leaving the object in a valid state. The tp_clear added for functions was not like that because other code expected structure fields to be non-NULL. At least that's my understanding. Is the behavior of tp_clear the key to this bug? In the original design GC, calling tp_clear was not guaranteed to cause objects in the cylce to be freed. So, after tp_clear, the object still needs to be in some kind of valid state. That is one of the reasons why the tuple type doesn't have a tp_clear (also that they can't normally be directly be used to create cycles). One last observation: the reference cycle GC is now being relied upon in ways that were never originally envisioned. Originally the cyclic GC was an optional feature and you could build Python without it. Reference cycles were supposed to exist in rare cases and only be created by user code. In that world, having tp_clear on the function type is be unneeded. Now, CPython is creating reference cycles itself and the GC is being relied on much more to free memory. I'm afraid we are evolving into a complicated and low performance GC (refcount + cycle collection). I don't have any good suggestions on a better approach. Maintaining the C-API ties us to reference counting. I mention it because it might be helpful to have a high-level view of the evolution of this part of CPython. -- nosy: +nascheme, tim.peters ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38009] Handle weakreference callbacks invoked indirectly in the middle of a gc collection
Neil Schemenauer added the comment: I think the problem with your logic is that the weakref to F is part of the garbage set. So, handle_finalizers() should detect that and clear the finalizer rather than call it. Once we get to delete_garbage() and start calling tp_clear(), we can't be running weakref callbacks or finalizers. The GC logic goes through great pains to ensure that. -- nosy: +nascheme ___ Python tracker <https://bugs.python.org/issue38009> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: Victor: > I'm surprised that the function would be seen as "unreachable" if > it's reference counter was equal to 135: If all those references are contained within that garbage set, it is possible. Pablo: > For the weakref to be handled correctly the ctypedescr needs to be > identified correctly as part of the isolated cycle. If is not in > the isolated cycle something may be missing tp_traverse or the GC > flags. I think this analysis is correct. I did some poking around with using 'rr'. It is very handy to run the program in reverse and use conditional breakpoints based on pointer addresses. The objects involved seem to be: CF: cfield CT: ctypedescr F: function W: weakref They have the following structure: CF -> CT -> W -> F The first link the chain is not found by the GC because CF does not implement tp_traverse. All those objects are part of garbage kept alive by the reference cycle and the GC calls delete_garbage() on it. CT is not part of 'unreachable' and handle_weakrefs() does not find W. What I previously said about tp_clear was incorrect. We take great pains to avoid running non-trivial code after calling tp_clear (no finalizers and no callbacks). So, the func_clear method should be safe. Even if tp_clear doesn't succeed in breaking the cycle, there should be no way for user Python code to access those objects that have had tp_clear called on them. Is gc.get_objects() a hole in this? Seems like you could cause crashes with it. This bug is surprising to me in another way. If the analysis is correct then fully implementing the GC protocols is no longer optional (as it was when cyclic GC was first introduced). If the GC cannot find garbage weakrefs handle_weakrefs() doesn't work and we end up with crashes like this. Maybe Pablos fix to not call the weakref callback if we are in delete_garbage() is a good enough band-aid. The alternative would seem to be ensuring that tp_traverse methods exist so that every single reference can be followed by the GC. I imagine that would be a huge task and there are many extension types that don't have them. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: I hacked up my copy of CPython to add flags to PyObject to see the GC state of objects. That makes it easier to know if an object is in the 'unreachable' list or not. > We must know that F is trash, else we never would have called tp_clear(F). Yup. In the crashing test program, F is in 'unreachable'. > We must NOT know CT is trash. If we did know that, then we would > have found W too, and then: Yes again. CT is not in 'unreachable'. > So if CT _is_ trash, we didn't know it. If/when delete_garbage > broke enough cyclea that CT's refcount fell to 0, other parts of > Python would have invoked W's callback as a matter of course, with > F possibly already tp_clear'ed. I haven't verified this but it must be what's happening. Note that my flags show that W *is* in 'unreachable'. It has to be otherwise F would not have tp_clear called on it. > Neil, at a high level it's not REALLY surprisimg that gc can screw > up if it can't tell the difference between trash & treasure ;-) > Long ago, though, it's possible the only real failure mode was > leaking objects that were actually unreachable. It has been so long I can't be sure the GC worked that way but I think it did. It would only call tp_clear on things it could really prove were garbage. If tp_clear didn't actually break the cycle, the only harmful result was leaking memory. That behavior is similar to a conservative GC. > Offhand not worried about get_objects(). That returns objects from > generation lists, but gc moves possible trash among temporary > internal (not generation) lists as it goes along. But when delete_garbage() gets done, it moves the object to 'old'. I think that means gc.get_objects() called after the collection completes can reveal objects that have had their tp_clear called (if tp_clear didn't result in them being freed). -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: Since W is in the unreachable set, we should not be executing its callback. Would the attached rough patch (gc_disable_wr_callback.txt) be a possible fix? When we find W inside handle_weakrefs(), we mark it as trash and will not execute the callback. This is similar to Pablo's bpo-38009 but I think attacks the problem in a better way. Having func_clear() being run is only one possible bad outcome of executing the callback. We want to avoid executing *any* user level Python code if the weakref has access to objects found by the GC and which have tp_clear called on them. -- Added file: https://bugs.python.org/file48630/gc_disable_wr_callback.txt ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > Fleshing out something I left implicit: if there's a trash object T > with a finalizer but we don't KNOW it's trash, we won't force-run its > finalizer before delete_garbage starts either. Then, really the same > thing: we may tp_clear some piece of trash T's finalizer needs before > enough cycles are broken that T's finalizer gets run as a routine > consequence of T's refcount falling to 0. Definition: a 'valid' object is one that hasn't had tp_clear called I think the difference is that non-weakref finalizers have strong references to objects that they can access when they run. So, if we haven't found them, they will keep all the objects that they refer to alive as well (subtract_refs() cannot account for those refs). So those objects will all be valid. There seems a hole though. Non-weakref finalizers could have a weakref (without callback) to something in the garbage set. Then, when the finalizer runs during delete_garbage(), that finalizer code can see non-valid objects via the weakref. I think this can only happen if there are missing/incomplete tp_traverse methods. We can have finalizer code running during delete_garbage(). That code should not have access to non-valid objects. Weakrefs seem be a way to violate that. handle_weakrefs() take care of some of them but it seems there are other issues. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > We can have finalizer code running during delete_garbage(). That > code should not have access to non-valid objects. Weakrefs seem be > a way to violate that. handle_weakrefs() take care of some of them > but it seems there are other issues. I see that handle_weakrefs() calls _PyWeakref_ClearRef() and that will clear the weakref even if it doesn't have callback. So, I think that takes care for the hole I was worried about. I.e. a __del__ method could have a weakref to an non-valid object. However, because handle_weakrefs() will find that weakref, it will have been cleared when the __del__ method executes. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Change by Neil Schemenauer : -- pull_requests: +16083 pull_request: https://github.com/python/cpython/pull/16495 ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > Why setting it back to release blocker? As far as I recall, this issue is not > a Python 3.8 regression. The regression which triggered this old and existing > bug has been fixed, see previous comments. I leave it up to our glorious release manager to decide how to proceed. IMHO, GH-15826 (revert the addition of tp_clear for functions) and GH-15641 (Avoid closure in weakref.WeakValueDictionary) are two cases of not fixing the real bug, just hiding symptoms. However, since this bug has existed for decades, I wouldn't fault him for taking the conservative approach and not trying to address the real bug in 3.8. I've created a new PR (GH-16083) that I think is the correct fix. If that PR is applied, I think we should also restore tp_clear for functions (revert GH-15826). GH-15641 can probably stay. Tim on my rough patch: > It's semantically correct since we never wanted to execute a callback from a > trash weakref to begin with Thanks for your help Tim. I think my PR is a cleaner fix that does essentially the same thing. Just call _PyWeakref_ClearRef() on weakrefs that are in 'unreachable'. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: Oops, I linked to wrong PR, my proposed fix is GH-16495. It is the same as what Tim suggests in his last comment. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > Would [func tp_clear] help with memory usage in functions or was BPO-33418 > addressed in another way since? Having a tp_clear for all container objects that can be involved in reference cycles will help the GC free memory. BPO-33418 may be contrived but it does demonstrate a problem that could happen in "real" code. If you have a long-running program, tracking down a memory leak due to reference cycles can be fiendishly difficult. That's why Tim originally wrote cyclops and why I started working on cyclic GC in the first place. It is a "quality of implementation" issue. It is not wrong for CPython to leak memory if you create reference cycles (not all types are required to implement the GC protocol). I expect users would be surprised if it happens since the majority have never used Python without the cyclic GC. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > Is introducing tp_clear on functions a thing that has ABI consequences? In > other words, if we take our time on this, would it land in 3.8.1 or 3.9.0? I think it should not have ABI consequences. However, I see the addition of tp_clear as a new feature. Leaking memory due to reference cycles is bad behavior but not a bug to be fixed in a maintenance release. Unless we require full GC protocol on every container object, we can't promise not to leak. Inaka's change to add func tp_clear has been in all the 3.8.0 pre-releases (I think). So, it should be pretty well exercised by now (at least, as well as the per-releases are tested). -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Change by Neil Schemenauer : -- pull_requests: +16092 pull_request: https://github.com/python/cpython/pull/16502 ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37725] "make clean" should remove PGO task data
Change by Neil Schemenauer : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue37725> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: I created GH-16502. I'm not exactly sure of the process of making a revert on a branch like '3.8' so hopefully it is correct. The code change is exactly what has been reverted in 3.8. The net effect will be as if the revert was never done. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Change by Neil Schemenauer : -- nosy: +benjamin.peterson, larry, ned.deily versions: +Python 2.7, Python 3.5, Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: I worked some on trying to create a unit test this evening. Attached is my best result so far (gc_weakref_bug_demo.py). It requires that you enable the 'xx' module so we have a container object without tp_traverse. We should probably add one to _testcapi for the real unit test so we have it. I can make the weakref callback run from within delete_garbage(). I haven't been able to make tp_clear for the function execute before the callback and I think that is needed to replicate the crash in this bug report. -- Added file: https://bugs.python.org/file48633/gc_weakref_bug_demo.py ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33418] Memory leaks in functions
Change by Neil Schemenauer : -- pull_requests: +16095 pull_request: https://github.com/python/cpython/pull/16502 ___ Python tracker <https://bugs.python.org/issue33418> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: Łukasz, is there some reason you removed old versions (2.7, 3.6, etc)? The bug is present on those versions of Python and it should be trivial to backport the fix. If those branches are maintained, I think we should fix it. Attached is a small test program (gc_weakref_bug_demo2.py) that causes the same crash as in this bug report (SEGV inside _PyFunction_Vectorcall). Setting things up is quite tricky and it depends on the order that things are cleared in delete_garbage(). Maybe still worth making a unit test for it? Instead of using the 'dummy' function, you can also use types.SimpleNamespace(). Calling repr() on it after tp_clear will result in a crash or an assert failure. Those two crashes are not needed to confirm the bug. Just the fact that 'callback' runs is proof enough. So, in a unit test, the callback to just set a global to indicate failure. -- Added file: https://bugs.python.org/file48637/gc_weakref_bug_demo2.py ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Change by Neil Schemenauer : -- pull_requests: +16348 pull_request: https://github.com/python/cpython/pull/16788 ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: > It's fundamentally insane to expect any gc to work perfectly when it may be > blind to what the object graph _is_. I'm often amazed it works at all, let alone perfectly. ;-P This did trigger me to think of another possible problem. I setup my unit test as you suggested: # Z <- Y <- A--+--> WZ -> C # ^ | # +--+ # where: # WZ is a weakref to Z with callback C # Y doesn't implement tp_traverse # A contains a reference to itself, Y and WZ But what happens if the GC doesn't see that WZ is trash? Then it will not be cleared. Hang it off Y so the GC can't find it. We can set things up so that Z is freed before WZ (e.g. WZ in a second and newer cycle). Then, the callback might still run. On further thought, this seems safe (but maybe broken) because of the handle_weakrefs() logic. The GC will think WZ is going to outlive Z so it will call it before doing any tp_clear calls. -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue38006] Crash in remove() weak reference callback of weakref.WeakValueDictionary at Python exit
Neil Schemenauer added the comment: New changeset 392a13bb9346331b087bcd8bb1b37072c126abee by Neil Schemenauer in branch 'master': bpo-38006: Add unit test for weakref clear bug (GH-16788) https://github.com/python/cpython/commit/392a13bb9346331b087bcd8bb1b37072c126abee -- ___ Python tracker <https://bugs.python.org/issue38006> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37448] obmalloc: radix tree for tracking arena address ranges
Neil Schemenauer added the comment: New changeset 85b6b70589c187639aeebc560d67e9cc04abb4d8 by Neil Schemenauer in branch 'master': bpo-37448: Use radix tree for pymalloc address_in_range(). (GH-14474) https://github.com/python/cpython/commit/85b6b70589c187639aeebc560d67e9cc04abb4d8 -- ___ Python tracker <https://bugs.python.org/issue37448> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37448] obmalloc: radix tree for tracking arena address ranges
Change by Neil Schemenauer : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue37448> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43593] pymalloc is not aware of Memory Tagging Extension (MTE) and crashes
Neil Schemenauer added the comment: I've merged PR 14474 so you can just test with an up-to-date "main" branch and see if that fixes the problem. I would expect it should fix the problem since with the radix tree arena tracking, no memory unsanitary behaviour remains. -- nosy: +nascheme ___ Python tracker <https://bugs.python.org/issue43593> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37368] test_asyncio: test_create_server_ssl_match_failed() failed on s390x SLES 3.x and logged an unraisable exception
Neil Schemenauer added the comment:
Seems this failure might be back. At least, the traceback looks quite similar
to me.
The buildbot failed with this:
heads/master:85b6b70589, Mar 29 2021, 22:53:15
0:05:26 load avg: 3.95 [426/427] test_tokenize passed (56.0 sec) -- running:
test_asyncio (44.8 sec)
0:05:35 load avg: 3.34 [427/427/1] test_asyncio failed (env changed) (54.1 sec)
Warning -- Unraisable exception
Exception ignored in:
Traceback (most recent call last):
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py",
line 321, in __del__
self.close()
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py",
line 316, in close
self._ssl_protocol._start_shutdown()
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py",
line 590, in _start_shutdown
self._abort()
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py",
line 731, in _abort
self._transport.abort()
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/selector_events.py",
line 680, in abort
self._force_close(None)
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/selector_events.py",
line 731, in _force_close
self._loop.call_soon(self._call_connection_lost, exc)
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/base_events.py",
line 745, in call_soon
self._check_closed()
File
"/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z/build/Lib/asyncio/base_events.py",
line 510, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
--
nosy: +nascheme
resolution: out of date ->
stage: resolved ->
status: closed -> open
versions: +Python 3.10 -Python 3.9
___
Python tracker
<https://bugs.python.org/issue37368>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37368] test_asyncio: test_create_server_ssl_match_failed() failed on s390x SLES 3.x and logged an unraisable exception
Neil Schemenauer added the comment:
It seems to not be specific to S390, same kind of failure on the PPC64LE RHEL8
LTO + PGE 3.x tester:
Exception ignored in:
Traceback (most recent call last):
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/sslproto.py",
line 321, in __del__
self.close()
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/sslproto.py",
line 316, in close
self._ssl_protocol._start_shutdown()
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/sslproto.py",
line 590, in _start_shutdown
self._abort()
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/sslproto.py",
line 731, in _abort
self._transport.abort()
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/selector_events.py",
line 680, in abort
self._force_close(None)
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/selector_events.py",
line 731, in _force_close
self._loop.call_soon(self._call_connection_lost, exc)
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/base_events.py",
line 745, in call_soon
self._check_closed()
File
"/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto-pgo/build/Lib/asyncio/base_events.py",
line 510, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
--
___
Python tracker
<https://bugs.python.org/issue37368>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43593] pymalloc is not aware of Memory Tagging Extension (MTE) and crashes
Neil Schemenauer added the comment: > If MTE is actually being used, system software assigns "random" values to 4 > of the higher-order bits. Oh, interesting. Two ideas about handling that: we could change our assertion check to be different on ARM platforms that we know have a certain size physical address space. Probably just turn off that high-bits check. Second idea, we could change the radix tree to not assume high address bits are unused. That's trickier to do without performance or memory usage degradations. I have a work-in-progress patch that adds a cache on top of the radix tree lookup. It looks like that cache can be made to have a pretty high hit rate. Based on a small amount of testing, the radix tree lookup for address_in_range() only happens about 1% of the time. If that approach works, we could add another layer to the tree and handle the full 64-bit address space. Based on my wip testing, my benchmark was showing about equal performance with the cache to without. So, no benefit to offset the increase in code complexity. Handling the MTE high bits tricks might enough to justify the cache addition. -- versions: +Python 3.9 -Python 3.8 ___ Python tracker <https://bugs.python.org/issue43593> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue42972] [C API] Heap types (PyType_FromSpec) must fully implement the GC protocol
Change by Neil Schemenauer : -- nosy: +nascheme ___ Python tracker <https://bugs.python.org/issue42972> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44331] Generate static PyCodeObjects for faster startup
New submission from Neil Schemenauer : Note: This is a proof of concept and not ready for merging as is. This is based on 'frozen_modules' from Jeethu Rao , via Larry Hastings. Larry's git branch was: g...@github.com:larryhastings/cpython.git not_another_use_of_the_word_frozen Usage: - Compile Python as normal - Run "make regen-freeze-startup" to re-generate Python/frozenmodules_code.c - Compile Python a second time Changes from Larry's branch: - Move static C code generation tool to Tools/freeze2 - Move _serializer to Modules - Rebase on Python 3.10.0b1 - determine startup modules by running sys.executable - use importlib.util.find_spec() to get code objects - fix ref-counting when setting __path__ - put static frozen modules in frozen_code_objects dict - reduce set of "bad" modules as it seems only _collections_abc needs exclusion - fix the is_frozen_package() and is_frozen() functions to find static frozen code It's not passing all unit tests yet but I'm somewhat hopeful there are no deep problems. Porting the changes from 3.6 to 3.8 and then to 3.10 was not too horrible. There was a few changes to PyGC_Head, to the PyCodeObject structure and to the runtime initialization process. That gives me some hope that it wouldn't be too burdensome to maintain this in the long-term. Mostly it would be updating _serialize.c to keep up with PyCodeObject changes. Based on benchmarking with 3.8, this gives a decent speedup for startup of a trival program, e.g. python -c "True". I measure it as taking 76% of the time. The savings are mostly in marshal.c but there is also some importlib/filesystem overhead that's removed. -- messages: 395243 nosy: nascheme priority: low severity: normal stage: patch review status: open title: Generate static PyCodeObjects for faster startup type: performance ___ Python tracker <https://bugs.python.org/issue44331> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44331] Generate static PyCodeObjects for faster startup
Change by Neil Schemenauer : -- keywords: +patch pull_requests: +25161 pull_request: https://github.com/python/cpython/pull/26571 ___ Python tracker <https://bugs.python.org/issue44331> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44523] weakref.proxy documentation might be outdated
Neil Schemenauer added the comment: It seems to me the old behaviour (don't forward hash) was done for good reasons. If the referent might go away, it is not valid to use it as a dict key since the hash and eq result changes. If it can't go away, what reason is there to use a weakref and not a direct reference? -- ___ Python tracker <https://bugs.python.org/issue44523> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43384] Include regen-stdlib-module-names in regen-all
Change by Neil Schemenauer : -- resolution: -> rejected stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue43384> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of GC_UNTRACK with TRASHCAN
New submission from Neil Schemenauer : The fix for bpo-33930 includes a somewhat mysterious comment: // The Py_TRASHCAN mechanism requires that we be able to // call PyObject_GC_UnTrack twice on an object. I wonder if we can just integrate the untrack into the body of the trashcan code. Then, the explicit call to untrack in the dealloc function body can be removed. That removes the risk of incorrectly using the macro version. I suspect the reason this was not done originally is because the original trashcan mechanism did not use the GC header pointers to store objects. Now, any object that uses the trashcan *must* be a GC object. -- messages: 399356 nosy: nascheme priority: normal severity: normal stage: needs patch status: open title: Consider integration of GC_UNTRACK with TRASHCAN type: enhancement ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33930] Segfault with deep recursion into object().__dir__
Neil Schemenauer added the comment: I'm thinking that the explicit call to PyObject_GC_UnTrack should be made unnecessary by integrating it into the trashcan code. That way, we avoid someone else running into this kind of bug in the future. See bpo-44881. -- nosy: +nascheme ___ Python tracker <https://bugs.python.org/issue33930> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of GC_UNTRACK with TRASHCAN
Change by Neil Schemenauer : -- keywords: +patch pull_requests: +26201 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/27718 ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of GC_UNTRACK with TRASHCAN
Neil Schemenauer added the comment: Extensions that call PyObject_GC_UnTrack before calling Py_TRASHCAN_BEGIN will still work, they will just take a very minor performance hit. I don't think it is worth the trouble to introduce new macros for that reason. Extensions that really care about performance can wrap the call in a Python version ifdef. There is an issue if someone writes and tests their extension with the new API, i.e. without having the explicit PyObject_GC_UnTrack() call in their dealloc method. If they compile with an older Python, they likely get a crash. If they compile with asserts enable, they would get an assert fail in _PyTrash_thread_deposit_object, i.e.: _PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op)); I guess that's an argument for new macros. -- stage: patch review -> needs patch ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of GC_UNTRACK with TRASHCAN
Neil Schemenauer added the comment: For examples of bugs caused by forgetting the untrack calls, see bpo-31095. -- ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of GC_UNTRACK with TRASHCAN
Neil Schemenauer added the comment:
Since C99 is now allowed, perhaps we should suggest that declarations go after
Py_TRASHCAN_BEGIN, e.g.
mytype_dealloc(mytype *p)
{
Py_TRASHCAN_BEGIN(p, mytype_dealloc)
... declarations go here ...
... The body of the deallocator goes here, including all calls ...
... to Py_DECREF on contained objects. ...
Py_TRASHCAN_END// there should be no code after this
}
The only dealloc method that doesn't fit this pattern is subtype_dealloc() and
that's because the object might not be a GC object.
Given the pattern, it occurs to me that that _Py_Dealloc() could do the
trashcan begin/end work. However, the authors of the dealloc methods would
have to realize the special rules of the trashcan (e.g. no returns from the
dealloc method body). Also, there would be some overhead added to
_Py_Dealloc() to check if the trashcan is supported. E.g. checking a type flag.
Another idea would be to add a new slot for the method, e.g. tp_dealloc_trash.
Then, _Py_Dealloc() could check if it exists and if so do the trashcan
begin/end dance around it. That would still add some overhead to _Py_Dealloc()
so I think the current macros are the best approach.
--
___
Python tracker
<https://bugs.python.org/issue44881>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of PyObject_GC_UnTrack() with the trashcan C API
Neil Schemenauer added the comment: I was thinking about this more today and I think the better fix is to actually build the trashcan mechanism into _Py_Dealloc(). Requiring that types opt-in to the trashcan mechanism by using the trashcan macros is not ideal. First, the trashcan macros are a bit tricky to use correctly. Second, every "container" type is potentially a part of a long ref chain and could blow up the stack on deallocation (i.e. triggered from DECREF). So, for correctness/robustness, every type that supports cyclic GC should get trashcan-style deallocation. We would have to find a way to do this without incurring a (significant) performance hit. Also, it would have to be done without breaking C extensions. Ideally they would get trashcan-style deallocation without any source code changes. I'm not sure if that can be done but I'm hopeful it's possible. -- ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44897] Integrate trashcan mechanism into _Py_Dealloc
New submission from Neil Schemenauer : This is a WIP/proof-of-concept of doing away with Py_TRASHCAN_BEGIN and Py_TRASHCAN_END and instead integrating the functionality into _Py_Dealloc. There are a few advantages: - all container objects have the risk of overflowing the C stack if a long reference chain of them is created and then deallocated. So, to be safe, the tp_dealloc methods for those objects should be protected from overflowing the stack. - the Py_TRASHCAN_BEGIN and Py_TRASHCAN_END macros are hard to understand and a bit hard to use correctly. Making the mechanism internal avoids confusion. The code can be slightly simplified as well. This proof-of-concept seems to pass tests but it will need some careful review. The exact rules related to calling GC Track/Untrack are subtle and this changes things a bit. I.e. tp_dealloc is called with GC objects already untracked. For 3rd party extensions, they are calling PyObject_GC_UnTrack() and so I believe they should still work. The fact that PyObject_CallFinalizerFromDealloc() wants GC objects to definitely be tracked is a bit of a mystery to me (there is an assert to check that). I changed the code to track objects if they are not already tracked but I'm not sure that's correct. There could be a performance hit, due to the _PyType_IS_GC() test that was added to the _Py_Dealloc() function. For non-GC objects, that's going to be a new branch and I'm worried it might hurt a bit. OTOH, maybe it's just in the noise. Profiling will need to be done. -- components: Interpreter Core messages: 399433 nosy: nascheme priority: normal severity: normal stage: patch review status: open title: Integrate trashcan mechanism into _Py_Dealloc type: enhancement ___ Python tracker <https://bugs.python.org/issue44897> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44897] Integrate trashcan mechanism into _Py_Dealloc
Change by Neil Schemenauer : -- keywords: +patch pull_requests: +26218 pull_request: https://github.com/python/cpython/pull/27738 ___ Python tracker <https://bugs.python.org/issue44897> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of PyObject_GC_UnTrack() with the trashcan C API
Neil Schemenauer added the comment: I wrote a proof-of-concept as bpo-44897. PR 27718 (this issue) might a slightly better performance but I like the other one better because it doesn't expose so much implementation detail to extension types. Either of them require careful review before merging. -- ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of PyObject_GC_UnTrack() with the trashcan C API
Neil Schemenauer added the comment:
> The problem of PyObject_GC_UnTrack() is just the most visible effect of the
> trashcan mecanism: tp_dealloc can be called twice, and this is *not* expected
> by the tp_dealloc API.
The fact that Py_TRASHCAN_BEGIN and Py_TRASHCAN_END can cause tp_dealloc to be
called more than once is not a huge issue, IMHO. The statements executed more
than once are the ones before or after the trashcan macros. If you use them
properly, the multiple calls doesn't cause issues. The big trap is
to use _PyObject_GC_UNTRACK before Py_TRASHCAN_BEGIN. It is not safe to call
that macro multiple times. Anywhere you see Py_TRASHCAN_BEGIN and
_PyObject_GC_UNTRACK before it, that's a bug.
The point of this PR is to make the macros harder to use incorrectly. If there
is no need to call untrack, Py_TRASHCAN_BEGIN and Py_TRASHCAN_END can normally
be the first and last lines of the tp_dealloc function (declarations can be
inside). Then, the fact that tp_dealloc is actually called more than once by
the
trashcan doesn't cause an issue.
tp_dealloc can get called more than once for another reason. If the object is
resurrected by tp_finalize or tp_del, then tp_dealloc can be called again.
It's pretty mind bending to try to understand all of implications of that. Most
of the ugliness is inside PyObject_CallFinalizerFromDealloc(). The type with
the finalizer has to be careful not to put the object in an invalid state
before it gets resurrected.
> Putting trashcan mecanism outside tp_dealloc can allow to make sure that
> tp_dealloc is called exactly once. _Py_Dealloc() sounds like a good
> candidate, but I didn't check if it's the only way to call tp_dealloc. Are
> there other places where tp_dealloc is called *directly*?
tp_dealloc is called from a few more places, as you found. As for providing
C-stack exhaustion protection via the trashcan, I think we are only interested
in places were tp_dealloc is called as a result of a DECREF. And I think that
only goes through _Py_Dealloc().
I suppose it would be possible to blow up the C stack via a call chain like:
subtype_dealloc -> basedealloc -> subtype_dealloc -> basedealloc -> ...
I think you would have to make a long chain of subclasses. Seems unlikely in
real code so I'm not worried about trying to fix that.
> Using military grade regex, I found the following functions calling
> tp_dealloc:
>
> grep 'dealloc[) ]*([a-zA-Z]\+)' */*.c
>
> * _Py_Dealloc() obviously
> * _PyTrash_thread_destroy_chain()
> * subtype_dealloc()
> * Modules/_testcapimodule.c: check_pyobject_freed_is_freed() <= unit test, it
> can be ignored
>
> So if we move the trashcan usage inside functions, 3 functions must be
> modified:
>
> * _Py_Dealloc()
> * _PyTrash_thread_destroy_chain()
> * subtype_dealloc()
Take a look at bpo-44897. It implements the _Py_Dealloc approach. You can see
what needs to be modified. I removed the Py_TRASHCAN_BEGIN/Py_TRASHCAN_END
macros as well, just because they are not needed anymore. _PyObject_GC_UNTRACK
calls inside tp_dealloc methods need to be removed because _Py_Dealloc already
does the untrack. For external extensions, they must be using
PyObject_GC_UnTrack() and so that's safe (object is already untracked and so it
does nothing).
I think the big change is calling tp_dealloc methods with the object already
untracked. If an extension did something like:
mytype_dealloc(PyObject *self)
{
...
assert(PyObject_GC_IsTracked(self));
...
}
That would break after PR 27738, at least as it's currently written.
The other issue I'm not quite sure about is that
PyObject_CallFinalizerFromDealloc insists that GC objects are tracked when that
function is called. I think it is okay to just call _PyObject_GC_TRACK on the
ones that are not tracked, to ensure that. Maybe Tim Peters will jump in and
correct the errors of my thinking.
--
nosy: +tim.peters
___
Python tracker
<https://bugs.python.org/issue44881>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44881] Consider integration of PyObject_GC_UnTrack() with the trashcan C API
Neil Schemenauer added the comment: > I think in any case we should benchmark this because this will affect *all* > GC types if I understand correctly and the TS mechanism had shown slowdowns > before We definitely need to benchmark. I would guess that adding trashcan protection to all GC types would not be a performance issue. We already had the macros for some performance critical ones (frame, tuple, list, dict). The performance hit will likely come as a result of adding a branch that uses _PyType_IS_GC() to the DECREF code path. It means any time an object hits zero refcount, we call _PyType_IS_GC() on it. Previously, we would just call tp_dealloc. Because of PEP 442, _PyType_IS_GC() checks not only a type flag but might also call tp_is_gc. So, benchmarks will need to be done. We might get a small win because the trashcan logic can be better optimized now that it's in a single complied unit. Small correction for my explaination above: if tp_dealloc gets called mutiple times because of the trashcan, it is the code before the BEGIN macro that gets called mutiple times. -- ___ Python tracker <https://bugs.python.org/issue44881> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
