Commit 87ddff6b authored by Colin Walters's avatar Colin Walters

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 0e36cdd5)
parent 96e785bb
......@@ -1077,6 +1077,16 @@ _dbus_get_is_errno_eintr (void)
return errno == EINTR;
}
/**
* 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)
......
......@@ -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);
......
......@@ -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
{
......@@ -806,6 +810,25 @@ do_reading (DBusTransport *transport)
return TRUE;
}
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,
......@@ -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);
......
......@@ -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.
......
......@@ -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);
/** @} */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment