msg58385 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2007-12-10 23:20 |
This adds signal.set_wakeup_fd(fd), which allows a single library to be
woken when a signal comes in.
|
msg58386 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2007-12-10 23:23 |
Guido, it looks like I can't alter the Assigned To field. You get the
Nosy List instead. ;)
|
msg58387 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-12-10 23:28 |
Thanks!
Can you add a doc patch too? Doc/library/signal.rst
|
msg58392 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2007-12-10 23:54 |
version 2, adds to Doc/library/signal.rst. It also tweaks the
set_wakeup_fd's docstring.
I haven't verified that my formatting in signal.rst is correct.
Specifically, the '\0' should be checked.
|
msg58438 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-12-11 16:27 |
``'\0'`` will be correct.
|
msg58442 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2007-12-11 17:25 |
Thanks georg.
|
msg58662 - (view) |
Author: Gustavo J. A. M. Carneiro (gustavo) * |
Date: 2007-12-15 14:50 |
The patch looks great.
But I was wondering if there is any chance to export a C API in addition
to the Python one? That is because PyGTK is mostly C, the code that
needs this is C, it would be easier to call a C API.
|
msg58663 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2007-12-15 17:35 |
The python API has the advantage that you can test for it at runtime,
avoiding a compile-time check. I don't know if this is significant though.
I don't see the big deal about a C API. All you need to do is call
PyImport_ImportModule("signal") and PyObject_CallMethod(mod,
"set_wakeup_fd", "i", fd);
|
msg58784 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-12-18 22:44 |
Here's an updated patch that applies smoothly to the current trunk.
|
msg58819 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-12-19 19:41 |
Committed revision 59574.
I added a simple C API as well, PySignal_SetWakeupFd(fd).
Thanks all!
|
msg339740 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-09 12:49 |
Why is this using type "sig_atomic_t" for a file descriptor instead of "int" (which is the type of file descriptors)? See https://github.com/python/cpython/pull/12670
|
msg339766 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2019-04-09 15:52 |
The fd field may be written from the main thread simultaneous with the signal handler activating and reading it out. Back in 2007 the only POSIX-compliant type allowed for that was sig_atomic_t, anything else was undefined.
Looks like pycore_atomic.h should have alternatives now but I'm not at all familiar with it.
|
msg339799 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-09 18:17 |
> Back in 2007 the only POSIX-compliant type allowed for that was sig_atomic_t, anything else was undefined.
Fair enough, but having a non-atomic type is still much better than a completely wrong type. In other words, the requirement of fitting a file descriptor is more important than being atomic.
|
msg339802 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2019-04-09 18:55 |
Disagree; if you're writing signal-handling code you should be very careful to do it properly, even if that's only proper for your current platform. If you can't do it properly you should find an alternative that doesn't involve signals.
The fact that sig_atomic_t is only 1 byte on VxWorks strongly implies using int WILL fail in strange ways on that platform. I can see three options:
1) use pycore_atomic.h, implementing it for VxWorks if you haven't already. This also implies sig_atomic_t could have been int but wasn't for some reason, such as performance.
2) disable wakeup_fd entirely. It's obscure, GNOME being the biggest user I can think of.
3) unpack the int into an array of sig_atomic_t. Only the main thread writes to it so this method is ugly but viable.
|
msg339805 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-09 20:40 |
I'm not sure with what you disagree. At least, you have to admit that using sig_atomic_t is buggy for different reasons than signal safety, namely that there is no guarantee that one can safely convert back and forth to an "int".
|
msg339806 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-09 20:41 |
> unpack the int into an array of sig_atomic_t.
What do you mean with this? You can't write a complete array atomically, so I don't see how this would help.
|
msg339807 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2019-04-09 21:03 |
Converting to/from sig_atomic_t could have a compile time check on currently supported platforms and isn't buggy for them. For platforms with a different size you could do a runtime check, only allowing a fd in the range of 0-254 (with 255 reserved); that could sometimes fail, yes, but at least it's explicit, easily understood failure. Just using int would fail in undefined ways down the road, likely writing to a random fd instead (corrupting whatever it was doing), with no way to trace it back.
Unpacking the int would mean having one sig_atomic_t for 'invalid', using that instead of INVALID_FD, plus an array of sig_atomic_t for the fd itself. Every time you want to change the fd you first set the 'invalid' flag, then the individual bytes, then clear 'invalid'.
|
msg339826 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-10 05:56 |
> Unpacking the int would mean having one sig_atomic_t for 'invalid', using that instead of INVALID_FD, plus an array of sig_atomic_t for the fd itself. Every time you want to change the fd you first set the 'invalid' flag, then the individual bytes, then clear 'invalid'.
I'm not sure that this is thread-safe as processors can reorder instructions, so there is no guarantee that memory is written in the expected order. That's one of the problems that C11/C++11 atomics solve.
|
msg339871 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2019-04-10 15:30 |
signal-safe is different from thread-safe (despite conceptual similarities), but regardless it's been a long time since I last delved into this so I'm quite rusty. I could be doing it all wrong.
|
msg339892 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-10 21:10 |
> signal-safe is different from thread-safe
I know, but I think that other threads can receive signals too. So in that case, it needs to be signal-safe as well as thread-safe.
|
msg339897 - (view) |
Author: Adam Olsen (Rhamphoryncus) |
Date: 2019-04-10 22:51 |
signalmodule.c has a hack to limit it to the main thread. Otherwise there's all sorts of platform-specific behaviour.
|
msg339962 - (view) |
Author: Jeroen Demeyer (jdemeyer) * |
Date: 2019-04-11 11:03 |
> signalmodule.c has a hack to limit it to the main thread.
The Python signal handler always runs in the main thread, but the signal can be caught by any thread. In other words, trip_signal() can be run by any thread.
|
msg339963 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-04-11 11:04 |
Please don't discuss on closed issues. Either reopen the issue or open a new issue.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:28 | admin | set | github: 45924 |
2019-04-11 11:04:48 | vstinner | set | nosy:
+ vstinner messages:
+ msg339963
|
2019-04-11 11:03:06 | jdemeyer | set | messages:
+ msg339962 |
2019-04-10 22:51:43 | Rhamphoryncus | set | messages:
+ msg339897 |
2019-04-10 21:10:33 | jdemeyer | set | messages:
+ msg339892 |
2019-04-10 16:00:48 | gvanrossum | set | nosy:
- gvanrossum
|
2019-04-10 15:30:41 | Rhamphoryncus | set | messages:
+ msg339871 |
2019-04-10 05:56:12 | jdemeyer | set | messages:
+ msg339826 |
2019-04-09 21:03:56 | Rhamphoryncus | set | messages:
+ msg339807 |
2019-04-09 20:41:03 | jdemeyer | set | messages:
+ msg339806 |
2019-04-09 20:40:09 | jdemeyer | set | messages:
+ msg339805 |
2019-04-09 18:55:56 | Rhamphoryncus | set | messages:
+ msg339802 |
2019-04-09 18:17:05 | jdemeyer | set | messages:
+ msg339799 |
2019-04-09 15:52:08 | Rhamphoryncus | set | messages:
+ msg339766 |
2019-04-09 12:49:04 | jdemeyer | set | nosy:
+ jdemeyer messages:
+ msg339740
|
2007-12-19 19:41:39 | gvanrossum | set | status: open -> closed resolution: accepted messages:
+ msg58819 |
2007-12-18 22:44:17 | gvanrossum | set | files:
+ python2.6-set_wakeup_fd4.diff messages:
+ msg58784 components:
+ Extension Modules |
2007-12-15 17:35:23 | Rhamphoryncus | set | messages:
+ msg58663 |
2007-12-15 14:50:32 | gustavo | set | nosy:
gvanrossum, georg.brandl, Rhamphoryncus, gustavo |
2007-12-15 14:50:13 | gustavo | set | nosy:
+ gustavo messages:
+ msg58662 |
2007-12-11 17:25:58 | Rhamphoryncus | set | files:
+ python2.6-set_wakeup_fd3.diff messages:
+ msg58442 |
2007-12-11 16:27:48 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg58438 |
2007-12-10 23:54:18 | Rhamphoryncus | set | files:
+ python2.6-set_wakeup_fd2.diff messages:
+ msg58392 |
2007-12-10 23:50:16 | gvanrossum | link | issue1564547 superseder |
2007-12-10 23:44:33 | gvanrossum | set | keywords:
+ patch, - py3k |
2007-12-10 23:44:25 | gvanrossum | set | priority: normal |
2007-12-10 23:28:28 | gvanrossum | set | keywords:
+ py3k assignee: gvanrossum messages:
+ msg58387 versions:
+ Python 2.6 |
2007-12-10 23:23:16 | Rhamphoryncus | set | nosy:
+ gvanrossum messages:
+ msg58386 |
2007-12-10 23:20:05 | Rhamphoryncus | create | |