diff options
author | Colin Walters <walters@verbum.org> | 2009-07-10 21:33:02 -0400 |
---|---|---|
committer | Colin Walters <walters@verbum.org> | 2009-07-14 15:41:57 -0400 |
commit | 87ddff6b24d9b9d4bba225c33890db25022d8cbe (patch) | |
tree | 6354abb89476866e7878c3f13a7d6b4b62a5f51b | |
parent | 96e785bb0614dc9ebbf6aebe12797d93a1b76b14 (diff) | |
download | dbus-87ddff6b24d9b9d4bba225c33890db25022d8cbe.tar.gz |
Bug 896 - Avoid race conditions reading message from exited process
Patch based on extensive work from Michael Meeks <michael.meeks@novell.com>,
thanks to Dafydd Harries <dafydd.harries@collabora.co.uk>,
Kimmo Hämäläinen <kimmo.hamalainen@nokia.com> and others.
The basic idea with this bug is that we effectively ignore errors
on write. Only when we're done reading from a connection do we
close down a connection. This avoids a race condition where
if a process (such as dbus-send) exited while we still had
data to read in the buffer, we'd miss that data.
(cherry picked from commit 0e36cdd54964c4012acec2bb8e598b85e82d2846)
-rw-r--r-- | dbus/dbus-sysdeps.c | 10 | ||||
-rw-r--r-- | dbus/dbus-sysdeps.h | 1 | ||||
-rw-r--r-- | dbus/dbus-transport-socket.c | 36 | ||||
-rw-r--r-- | dbus/dbus-watch.c | 6 | ||||
-rw-r--r-- | dbus/dbus-watch.h | 1 |
5 files changed, 46 insertions, 8 deletions
diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index e0fe8888..ccd80ccd 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -1078,6 +1078,16 @@ _dbus_get_is_errno_eintr (void) } /** + * See if errno is EPIPE + * @returns #TRUE if errno == EPIPE + */ +dbus_bool_t +_dbus_get_is_errno_epipe (void) +{ + return errno == EPIPE; +} + +/** * Get error message from errno * @returns _dbus_strerror(errno) */ diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 2fd54214..8ce6566d 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -362,6 +362,7 @@ dbus_bool_t _dbus_get_is_errno_nonzero (void); dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (void); dbus_bool_t _dbus_get_is_errno_enomem (void); dbus_bool_t _dbus_get_is_errno_eintr (void); +dbus_bool_t _dbus_get_is_errno_epipe (void); const char* _dbus_strerror_from_errno (void); void _dbus_disable_sigpipe (void); diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c index 46cbed96..8be4d135 100644 --- a/dbus/dbus-transport-socket.c +++ b/dbus/dbus-transport-socket.c @@ -616,7 +616,11 @@ do_writing (DBusTransport *transport) { /* EINTR already handled for us */ - if (_dbus_get_is_errno_eagain_or_ewouldblock ()) + /* For some discussion of why we also ignore EPIPE here, see + * http://lists.freedesktop.org/archives/dbus/2008-March/009526.html + */ + + if (_dbus_get_is_errno_eagain_or_ewouldblock () || _dbus_get_is_errno_epipe ()) goto out; else { @@ -807,6 +811,25 @@ do_reading (DBusTransport *transport) } static dbus_bool_t +unix_error_with_read_to_come (DBusTransport *itransport, + DBusWatch *watch, + unsigned int flags) +{ + DBusTransportSocket *transport = (DBusTransportSocket *) itransport; + + if (!(flags & DBUS_WATCH_HANGUP || flags & DBUS_WATCH_ERROR)) + return FALSE; + + /* If we have a read watch enabled ... + we -might have data incoming ... => handle the HANGUP there */ + if (watch != transport->read_watch && + _dbus_watch_get_enabled (transport->read_watch)) + return FALSE; + + return TRUE; +} + +static dbus_bool_t socket_handle_watch (DBusTransport *transport, DBusWatch *watch, unsigned int flags) @@ -817,14 +840,11 @@ socket_handle_watch (DBusTransport *transport, watch == socket_transport->write_watch); _dbus_assert (watch != NULL); - /* Disconnect in case of an error. In case of hangup do not - * disconnect the transport because data can still be in the buffer - * and do_reading may need several iteration to read it all (because - * of its max_bytes_read_per_iteration limit). The condition where - * flags == HANGUP (without READABLE) probably never happen in fact. + /* If we hit an error here on a write watch, don't disconnect the transport yet because data can + * still be in the buffer and do_reading may need several iteration to read + * it all (because of its max_bytes_read_per_iteration limit). */ - if ((flags & DBUS_WATCH_ERROR) || - ((flags & DBUS_WATCH_HANGUP) && !(flags & DBUS_WATCH_READABLE))) + if (!(flags & DBUS_WATCH_READABLE) && unix_error_with_read_to_come (transport, watch, flags)) { _dbus_verbose ("Hang up or error on watch\n"); _dbus_transport_disconnect (transport); diff --git a/dbus/dbus-watch.c b/dbus/dbus-watch.c index 7ef27bf0..bca699fd 100644 --- a/dbus/dbus-watch.c +++ b/dbus/dbus-watch.c @@ -51,6 +51,12 @@ struct DBusWatch unsigned int enabled : 1; /**< Whether it's enabled. */ }; +dbus_bool_t +_dbus_watch_get_enabled (DBusWatch *watch) +{ + return watch->enabled; +} + /** * Creates a new DBusWatch. Used to add a file descriptor to be polled * by a main loop. diff --git a/dbus/dbus-watch.h b/dbus/dbus-watch.h index fd65ae36..fa953ec1 100644 --- a/dbus/dbus-watch.h +++ b/dbus/dbus-watch.h @@ -74,6 +74,7 @@ void _dbus_watch_list_remove_watch (DBusWatchList *watch_li void _dbus_watch_list_toggle_watch (DBusWatchList *watch_list, DBusWatch *watch, dbus_bool_t enabled); +dbus_bool_t _dbus_watch_get_enabled (DBusWatch *watch); /** @} */ |