After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 743052 - use GUnixSignalWatchSource rather than sigwait()
use GUnixSignalWatchSource rather than sigwait()
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-01-16 16:37 UTC by Dan Winship
Modified: 2015-02-25 14:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Dan Winship 2015-01-16 16:37:22 UTC 
danw/signals-bgoXXXXXX

Someone forgot to call nm_unblock_posix_signals() in some patch somewhere recently, and I got annoyed and decided to kill it.

As a bonus, it turns out that this also fixes the bug where hitting ^C in gdb kills NetworkManager. Yay!

Having to modify all the child setup functions then led me to streamline our use of setpgid() a bit, which is not actually related to signals at all, but I was there...
Comment 1 Dan Williams 2015-01-16 17:01:04 UTC 
LGTM, nice cleanups
Comment 2 Lubomir Rintel 2015-01-19 12:55:17 UTC 
Works fine for me, looks good.
Comment 3 Jiri Klimes 2015-01-19 13:57:09 UTC 
Pushed a fixup. The code looks fine.
(I hope it doesn't have negative consequences e.g. with loosing signals due to theading, etc.)
Comment 4 Thomas Haller 2015-01-19 14:17:23 UTC 
LGTM
Comment 5 Dan Winship 2015-01-19 14:45:20 UTC 
(In reply to comment #3)
> (I hope it doesn't have negative consequences e.g. with losing signals due to
> theading, etc.)

The glib code handles multiple threads fine.

One possible source of bugs though is that there might be places in NM where we ought to have been handling EINTR, but weren't. With the old code, signals were masked in most threads, so they'd never get an EINTR, but with the new code they might.
Comment 6 Dan Winship 2015-01-19 16:50:53 UTC 
merged jklimes's fixup and pushed