This started with a report of ls giving a surprising diagnostic, due to some "other" code ignoring SIGPIPE. Here's the end of the thread: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/11203/focus=11902 It affected only those with a tcsh login shell, and only when getting a session via /bin/login (i.e., not via ssh). I tracked it down: mingetty exec's /bin/login, which uses pam library code, which in turn uses libdbus. And that's the culprit: signal (SIGPIPE, SIG_IGN) is called by default, and the prior handler is never restored. I saw that this behavior was introduced a long time ago: http://www.redhat.com/archives/message-bus-list/2003-February/msg00001.html but now, you can use sendto's POSIX-specified MSG_NOSIGNAL flag (at least on modern systems) to avoid having to change signal handlers altogether. If/when libdbus does have to make an offering to the gods of portability by ignoring signals for e.g., a sendto call, then please make it restore the previous handler right afterwards. libdbus is a library, after all, and libraries should try hard not to modify process-wide state like signal handlers. This is with dbus-1.1.2-7.fc8.
While I certainly agree that in general libraries should not modify global state, in this case the POSIX API is broken in a way that makes that (afaik) impossible; we can't have apps crashing when the other end of the socket closes, at least not by default. So, we provide a function to keep libdbus from changing global SIGPIPE state, for the rare app that needs to do something special on SIGPIPE. But by default, we do what 99% of apps will want, which is to ignore it. We can't set to SIG_IGN temporarily then put it back, because that is not threadsafe, afaik. An additional constraint here of course is backward compatibility; it would not be compatible if dbus-using apps suddenly had to set SIG_IGN on SIGPIPE themselves, or risk crashing. If there is a solution, such as using sendto(), then I'm open to it; problems with sendto() seem to include that it isn't portable to all platforms, and might be slower since we're using writev() to send multiple buffers at once. Note that we could not use sendto() on one platform and leave SIGPIPE alone, but modify SIGPIPE on another platform, because then apps would not know what to do (i.e. the API would be different on each platform). So, if we were to not touch SIGPIPE in favor of using sendto(), we'd have to be able to use MSG_NOSIGNAL on all relevant platforms (Linux, Solaris, BSD-flavors at minimum). Overall I'm not optimistic that there's a better solution than what we have; but again, I am open to anything that is backward compatible, gives uniform behavior across platforms, is threadsafe, and does not mean apps will crash by default when the other end of the connection drops. In the meantime I think PAM modules just need to call our "opt out" function to keep libdbus from ignoring SIGPIPE. But keep in mind, this means the module is going to crash if the other end of the socket is dropped, which may or may not be acceptable. I don't know how to avoid this.
> We can t set to SIG_IGN temporarily then put it back, because that is not > threadsafe, afaik. Well, libdbus throws thread safety out the window the moment it changes the SIGPIPE signal handler, so there s no additional penalty for restoring it later. The point about thread safety is that one thread should not be able to affect another, and changing sighandlers definitely affects other threads. If we assume that libdbus must ignore SIGPIPE, then there should be a standard way to tell libdbus to close its socket and *then* to restore to original state any signal handler it may have modified. Then, any library like PAM or application that calls this function can have some confidence that the library will not have changed global process attributes. An alternative would be to give enough information to the caller so that the caller can restore the state of any modified signal handlers. But it is very ugly to require a calling library like PAM to perform a non-thread-safe operation like that to clean up after libdbus. It s better to keep it all inside libdbus. Then, if/when it s ever fixed, the required changes will be relatively confined.
With the current dbus setup, thread safety works as follows: - if your app is not going to change SIGPIPE handler, there is no issue; only libdbus will change it - if your app is going to change it, you tell libdbus never to change it; so there is no issue If libdbus did temporary changes to SIGPIPE, there is an issue in the second case. - if your app is not going to change SIGPIPE, there is still no issue - if your app is going to change SIGPIPE, then libdbus will unpredictably disable your handler at various times - there is no way to code an app to deal with this IMO, so the correct thing would still be to have a call to tell libdbus never to touch SIGPIPE, which is what we already have Again, while I agree with you that a nicer solution would be good, I still don't see what that nicer solution would be. The SIGPIPE signal handler API is broken because it's process-global state, yet to do certain things, code MUST change the process-global state; ergo, we get in a situation where we are required to mess with process-global state. I don't see a way around that. Sometimes, things just aren't as optimal as we might like.
Thread-safety would be nice, but it isn t the issue here. All I want is a clean way to restore the pre-libdbus global state, once the socket is closed. Please reread my proposal, or at least this: libdbus should provide an interface so that code like what s in libpam can use dbus temporarily, and then close everything, with *no net change* in the caller s global state, and no risk of death by SIGPIPE. Sure, every libdbus-user can do something gross like record the pre-dbus sigpipe handler, and, if needed, restore it after closing the dbus socket, but I think dbus should provide an easy way to do this, so that a caller that cares doesn t have to perform such contortions.
> libdbus should provide an interface so that code like what s in libpam > can use dbus temporarily, and then close everything, with *no net change* > in the caller s global state, and no risk of death by SIGPIPE. Why isn't that possible now? We provide a function to tell libdbus not to change the SIGPIPE handler, ever. That means you can use libdbus temporarily (or permanently), and it will not modify global state. If you don't want to risk death by SIGPIPE in this case, then you have to install your own SIGPIPE handler or set SIGPIPE to SIG_IGN yourself. I don't understand what other API we could offer. We allow the app to say "don't touch SIGPIPE." If you have another API proposal, you'll have to more specifically spell out what functions you think libdbus should have.
As I said, an application *can* do that, but it should not have to. Considering the existence of MSG_NOSIGNAL-enabled functions like sendto, I hope libdbus will someday not need to change signal handlers. As such, it'd be best not to require that libdbus callers do things like save/restore signal handlers to accommodate the current implementation. I'm hoping for a way to keep the work-around code inside libdbus. Then, when the current POSIX-induced bogosity is fixed, only libdbus will have to change, rather than all callers. I proposed that libdbus record the original SIGPIPE handler (which may well be something other than SIG_IGN), and provide a mechanism for restoring that value, just after closing the dbus socket.
> Considering the existence of MSG_NOSIGNAL-enabled functions like sendto, > I hope libdbus will someday not need to change signal handlers. I hope for that too, but as I noted there are some questions to resolve around using sendto(). If someone addressed those and provided a patch, we'd certainly be open to it. My guess, however, is that MSG_NOSIGNAL is not portable enough. > I'm hoping for a way to keep the work-around code inside libdbus. Sure, if possible. The nature of global state, of course, is that it's tough to encapsulate. > I proposed that libdbus record the original SIGPIPE handler (which > may well be something other than SIG_IGN), and provide a mechanism > for restoring that value, just after closing the dbus socket. If you work out the exact API for this I'm not sure it's all that appealing; one thing that may not be obvious is that dbus does not have a single socket, it lets people create as many DBusConnection as they want. The "default" DBusConnection to the session or system bus is global to the process and is never closed, though the app need not open this default global connection, it can also create a close-able private connection. We could save the signal handler when the first connection opens, and then restore the signal handler that was installed at that time when the last connection closes, but I don't know if that's an improvement - it may just make it more unpredictable for apps that think about SIGPIPE handling themselves, while doing nothing for normal apps (since almost all apps open the global connection that never closes, anyway). It's also unclear that this would be a backward-compatible change. I'm not trying to be difficult, I just don't see an obviously better behavior or clear fix here. I would encourage for now just using dbus_connection_set_change_sigpipe() in PAM, unless someone is up for digging in to sendto()'s behavior on various platforms.
It looks as though we now use MSG_NOSIGNAL on OSs that have it (in theory since 1.4.1; there were some bugs in that check, which are fixed in git awaiting a 1.4.4 release). Havoc has also explained why we'll have to keep ignoring SIGPIPE on OSs that don't have MSG_NOSIGNAL (unless instructed otherwise by an application that knows better). With those in mind, can this bug be considered resolved?
I presume that means libdbus no longer changes the signal mask on systems with MSG_NOSIGNAL support. That's precisely what I requested 3 years ago, so you're welcome to close this. Thanks.
(In reply to comment #9) > I presume that means libdbus no longer changes the signal mask on systems with > MSG_NOSIGNAL support. Yes. Closing, thanks! Fixed in 1.4.4.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.