[issue13697] python RLock implementation unsafe with signals
Changes by Nikita Kovaliov nik...@maizy.ru: -- nosy: +maizy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Roumen Petrov added the comment: STINNER Victor wrote: [SNIP]I attach hang2.py which doesn't force the Python implementation of RLock.[SNIP] Ok. Fine with me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Antoine Pitrou added the comment: This bug has existed for ages. People who want sane behaviour should just switch to Python 3. Closing. -- resolution: - rejected stage: needs patch - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
STINNER Victor added the comment: The attached test script demonstrates the issue on Python 2.6 and 3.2, and code inspection suggests this is still valid for 2.7 and 3.4. I disagree that Python 3.4 is affected: RLock has been reimplemented in C in Python 3.2. Only the Python implementation of RLock has the bug, but it's not used by fault, you have to explicitly use a private attribute of the threading module to get it. Python 2.6 only accept security fixes, so in fact only Python 2.7 may get a fix. The question is if it worth to backport 300 lines of C code from Python 3 to Python 2 in the _thread module. @Benjamin, release manager of Pythn 2.7: What do you think? I may help to backport the C implementation of RLock. -- nosy: +benjamin.peterson versions: -Python 3.2, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Roumen Petrov added the comment: hmm issue still exist in master branch. Lets wait python 4 for sane behaviour. -- nosy: +rpetrov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
STINNER Victor added the comment: hmm issue still exist in master branch. For the third time, only the Python implementation has the bug, and it's not used by default. So the bug was fixed in Python 3 since 3.2. It's time to upgrade guys ;-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Roumen Petrov added the comment: STINNER Victor wrote: For the third time, only the Python implementation has the bug, and it's not used by default. So the bug was fixed in Python 3 since 3.2. It's time to upgrade guys ;-) Did you mean to downgrade? Tested with Python 3.5.0a1+ (default, Feb 28 2015, 09:49:09) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
STINNER Victor added the comment: Did you mean to downgrade? Tested with Python 3.5.0a1+ (default, Feb 28 2015, 09:49:09) Please make some effort to try to understand the issue. I attach hang2.py which doesn't force the Python implementation of RLock. You might try hang2.py on Python 2 and Python 3 if you are not convinced yet that Python 3 has not the bug... -- Added file: http://bugs.python.org/file38318/hang2.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
STINNER Victor added the comment: FYI I proposed a fix for eventlet to fix eventlet with Python 3 when monkey-patching is used: https://github.com/eventlet/eventlet/pull/187 The change forces the Python implementation of RLock, which is compatible with eventlet monkey-patching. The Python implementation of RLock gets the thread identifier from the monkey-patched threading module, whereas the C implementation calls directly a C function to get the identifier which is incompatible with eventlet. If the Python implementation is simply dropped, eventlet may uses its own implementation (copy/paste code from older Python version). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Lothsahn added the comment: I am using Python 2.6.5 (we will be upgrading to Python 2.7.9 soon) and I recently ran into this bug. If I do any locking in a signal handler with RLocks, the entire system can deadlock. I'm using this to serialize my IO so we don't have mismatched lines in our logs. It looks like RLock was implemented in C in 3.2. While I don't care about the performance benefits of that rewrite, having a non-deadlocking RLock implementation would be nice. Not sure if this issue can be fixed in 2.7, but it would be nice. C RLock implementation here: http://bugs.python.org/issue3001 -- nosy: +Lothsahn ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Changes by Chris Rebert pyb...@rebertia.com: -- nosy: +cvrebert ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Mike Meyer m...@mired.org added the comment:
I just ran into this issue in the logging module using 2.7. Here's the TB in
case it sheds any light on the issue
Traceback (most recent call last):
File ./crawler.py, line 531, in module
main(argv[1:]1:)
File ./crawler.py, line 522, in main
MCP(config).run()
File ./crawler.py, line 332, in run
self.reaper()
File ./crawler.py, line 359, in reaper
logging.debug('MCP process alive: %s', state)
File /usr/local/lib/python2.7/logging/__init__.py, line 1600, in debug
root.debug(msg, *args, **kwargs)
File /usr/local/lib/python2.7/logging/__init__.py, line 1120, in debug
self._log(DEBUG, msg, args, **kwargs)
File /usr/local/lib/python2.7/logging/__init__.py, line 1250, in _log
self.handle(record)
File /usr/local/lib/python2.7/logging/__init__.py, line 1260, in handle
self.callHandlers(record)
File /usr/local/lib/python2.7/logging/__init__.py, line 1300, in
callHandlers
hdlr.handle(record)
File /usr/local/lib/python2.7/logging/__init__.py, line 746, in handle
self.release()
File /usr/local/lib/python2.7/logging/__init__.py, line 700, in release
self.lock.release()
File /usr/local/lib/python2.7/threading.py, line 147, in release
self.__block.release()
thread.error: release unlocked lock
Since I'm not using threads, getting thread errors was a little bit of
a surprise. I guess trying to make the logging module thread safe
added a potential bug.
--
nosy: +Mike.Meyer
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13697
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Charles-François Natali neolo...@free.fr added the comment: Hmm, but that would break single-threaded programs which expect their select() (or other) to return EINTR when a signal is received (which is a perfectly valid expectation in that case). Yes, that's why I said thats another story ;-) EINTR is really a pain, and relying on it to return from a syscall upon signal reception is a bad idea (certain OS restart syscalls by default - not select() though - and if the signal is received before you call the syscall, you'll deadlock). This would IMHO be the best way to go, but I know we can't reasonably change this now. I don't know if that's still useful to build Python without threads. I would expect most platforms to have a compatible threads implementation (and Python probably can't run on very small embedded platforms). Perhaps you can ask on python-dev. There are another problems, for example it's very well known that signals and threads don't mix well (so for example you'd have to block all signals before spawning the new thread, and reenable them afterwards). I'm not sure it's worth the extra complication. I can still try to write a quick patch to see if it gets somewhere (and doesn't break test_threading and test_signals). The first and most simple thing we could do would be to nuke the Python version (and also the loggin hack at the same time): does that sound reasonable ? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Antoine Pitrou pit...@free.fr added the comment: The first and most simple thing we could do would be to nuke the Python version (and also the loggin hack at the same time): does that sound reasonable ? To me, yes, but I think it's better to ask on python-dev as for nuking the Python version. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Antoine Pitrou pit...@free.fr added the comment: That sounds like a good solution in the middle-term. Are there any drawbacks? (apart from launching a thread) Just to be clear: the approach I was suggesting is to have a resident thread dedicated to signal management, not to spawn a new one when needed. Another advantage is that we could mask signals in all threads except this one, and have a consistent cross-platform behavior with respect to signals+threads. Hmm, but that would break single-threaded programs which expect their select() (or other) to return EINTR when a signal is received (which is a perfectly valid expectation in that case). However I see two drawbacks: - it seems that we want to allow building Python without threads support. In that case, this wouldn't work, or we would need the current implementation as a fallback, but this would complicate the code somewhat. I don't know if that's still useful to build Python without threads. I would expect most platforms to have a compatible threads implementation (and Python probably can't run on very small embedded platforms). Perhaps you can ask on python-dev. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Antoine Pitrou pit...@free.fr added the comment: Yes, using synchronization primitives or doing I/O in Python signal handlers isn't a good idea. Perhaps the signal docs should be clearer about that. Of course, this wasn't ever safe code, and we're changing it (to have the signal handler merely set a integer flag that the logging handler can consult without locking) Indeed, setting a variable (or using set_wakeup_fd()) is the right approach. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Charles-François Natali neolo...@free.fr added the comment: The core of the problem is that we don't just want those methods to be atomic or thread-safe, but reentrant (or rather async-safe). As such, protecting by a lock isn't enough (and it's not really feasible in Python). Note that the RLock implementation already checks whether the lock is already acquire by the current thread, but there's a race: if self._owner == me: [...] self._count = self._count + 1 return 1 rc = self._block.acquire(blocking, timeout) if rc: self._owner = me If the signal handler is called after _block has been acquired, but before self._owner is set, the next call to acquire from the signal handler won't realize that the block has already been acquired by the current thread, and will deadlock. Now, the fun part: this affects not only RLock, but every Python code performing atomic actions: condition variables, barriers, etc. There are some constraints on what can be done from a signal handler, and it should probably be documented. Note that another solution would be to use a dedicated thread for signal management (like Java does), but that's another story. Also, this shouldn't be a problem for the buffered I/O code, since the code already accounts for this possibility (if the lock is already acquired by the current thread, an exception is raised). Now, there's something I don't understand: I've just had a quick look, but AFAICT, there's no reason why the C version of RLock could not be available: am I missing something? Why do we even have a Python implementation? -- nosy: +neologix ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Antoine Pitrou pit...@free.fr added the comment: Note that another solution would be to use a dedicated thread for signal management (like Java does), but that's another story. That sounds like a good solution in the middle-term. Are there any drawbacks? (apart from launching a thread) Also, this shouldn't be a problem for the buffered I/O code, since the code already accounts for this possibility (if the lock is already acquired by the current thread, an exception is raised). Yes, but raising an exception is less helpful than doing what the user asked for :) Now, there's something I don't understand: I've just had a quick look, but AFAICT, there's no reason why the C version of RLock could not be available: am I missing something? Why do we even have a Python implementation? The C version is quite recent, and there's a school of thought that we should always provide fallback Python implementations. (also, arguably a Python implementation makes things easier to prototype, although I don't think it's the case for an RLock) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Charles-François Natali neolo...@free.fr added the comment: That sounds like a good solution in the middle-term. Are there any drawbacks? (apart from launching a thread) Just to be clear: the approach I was suggesting is to have a resident thread dedicated to signal management, not to spawn a new one when needed. Another advantage is that we could mask signals in all threads except this one, and have a consistent cross-platform behavior with respect to signals+threads. However I see two drawbacks: - it seems that we want to allow building Python without threads support. In that case, this wouldn't work, or we would need the current implementation as a fallback, but this would complicate the code somewhat. - the thread would have to be respawned after a fork() The C version is quite recent, and there's a school of thought that we should always provide fallback Python implementations. (also, arguably a Python implementation makes things easier to prototype, although I don't think it's the case for an RLock) Hmmm, I'm not convinced by this argument in this specific case: since the C version is now the default, is faster and more reliable, can't we drop the Python version? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
STINNER Victor victor.stin...@haypocalc.com added the comment: This affects the python implementation of RLock only. In the issue #13550, it was discussed to remove completly the logging machinery from the threading module. If we remove it, we don't need the Python implementation of RLock. We already removed the Python implementation of subprocess.Popen._execute_child() in Python 3.3 because of issues with fork, signals and threads. -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Robert Collins robe...@robertcollins.net added the comment: Normally I advocate very strongly for Python implementation of C accelerated modules, but when the two implementations are not equivalent, having a simpler Python one around does not help anyone (not users, other language implementors etc). True reentrancy is possible but quite hard to achieve. So, FWIW, +1 on just having a C implementation. The dedicated signal thread sounds useful too - it would ease the issues for folk using other parts of the stdlib, and would in principle permit a pure-python RLock to hang around. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Changes by Alex Gaynor alex.gay...@gmail.com: -- nosy: +alex ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
New submission from Robert Collins robe...@robertcollins.net: This affects the python implementation of RLock only. If a signal occurs during RLock.acquire() or release() and then operates on the same lock to acquire() or release() it, process hangs or assertions can be triggered. The attached test script demonstrates the issue on Python 2.6 and 3.2, and code inspection suggests this is still valid for 2.7 and 3.4. To use it, run the script and then 'kill -SIGUSR2' the process id it prints out. Keep sending SIGUSR2 until you get bored or: - a traceback occurs - the process stops printing out '.'s We found this debugging application server hangs during log rotation (https://bugs.launchpad.net/launchpad/+bug/861742) where if the thread that the signal is received in was about to log a message the deadlock case would crop up from time to time. Of course, this wasn't ever safe code, and we're changing it (to have the signal handler merely set a integer flag that the logging handler can consult without locking). However, I wanted to raise this upstream, and I note per issue #3618 that the 3.x IO module apparently includes a minimal version of the Python RLock implementation (or just uses RLock itself). Either way this bug probably exists for applications simply doing IO, either when there is no C RLock implementation available, or all the time (depending on exactly what the IO module is doing nowadays). -- components: Library (Lib) files: hang.py messages: 150477 nosy: rbcollins priority: normal severity: normal status: open title: python RLock implementation unsafe with signals versions: Python 2.7, Python 3.4 Added file: http://bugs.python.org/file24126/hang.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Jesús Cea Avión j...@jcea.es added the comment: Adding myself to the nosy list because I am interested in the issue, but I can't take care of this myself for a few weeks, at least. Robert, would you consider to try to write a patch? -- nosy: +jcea versions: +Python 3.3 -Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Changes by Antoine Pitrou pit...@free.fr: -- stage: - needs patch type: - behavior versions: +Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Robert Collins robe...@robertcollins.net added the comment: I'm not sure it is sensibly implementable in pure python: the semantics of signal handling (AIUI) are that the vm is interrupted, sets a flag to say 'when the GIL is released or the next bytecode interpretation happens, please process signal X' [the former for C extensions, the latter for regular code], and then the OS level signal handler returns. The current C or bytecode execution completes and then we dispatch to the python signal handler. Now, what we would want here is that attempts to acquire/release the RLock in a signal handler would behave as though the RLock.acquire/release methods were atomic. The general way to enforce such atomicity is via a lock or mutex. Now, because the call stack looks like this: signalthread.acquire() non-signal-codethread.acquire() The other acquire, if it already holds this supposed mutex, would block the python signal handler acquire call from obtaining it; The inner acquire would have to error *whether or not a timeout was passed*, because the non-signal-code won't progress and complete until the python signal handler code returns. One could, in principle, use stack inspection to determine how far through a nested acquire/release the outer code is, but that seems, uhm, pathological to me :). The crux of the problem is detecting whether the non-reentrant thread.lock is held because (a) another thread holds it or (b) this thread holds it. If we can inspect its internals, we could determine that this thread holds it - and thus we must be reentered into the acquire/release methods. With that tricky thing established, we could look at the remaining logic to make sure that each line is either atomic or conditional on reentrancy. Simpler I think to require CRLocks always, and provide a portable C implementation which will always complete before returning to execution in the calling thread, avoiding this issue entirely. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13697] python RLock implementation unsafe with signals
Changes by Meador Inge mead...@gmail.com: -- nosy: +meador.inge ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13697 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
