From 4c95a9782c65f88e2904c44abeb734a1b00f6353 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sat, 15 Mar 2003 02:19:02 +0000 Subject: [PATCH] 2003-03-14 Havoc Pennington * dbus/dbus-memory.c: add a "detect buffer overwrites on free" cheesy hack * dbus/dbus-transport-debug.c: rework this a good bit to be less complicated. hopefully still works. * dbus/dbus-server-debug.c (handle_new_client): remove timeout manually * glib/dbus-gmain.c (timeout_handler): don't remove timeout after running it * dbus/dbus-message.c (dbus_message_copy): rename from dbus_message_new_from_message, fix it up to copy all the message fields, add test case * bus/dispatch.c (bus_dispatch_test): add some more test code, not quite passing yet --- ChangeLog | 21 ++++ bus/bus.c | 5 +- bus/connection.c | 11 +- bus/dispatch.c | 45 ++++++-- bus/loop.c | 29 +++-- bus/test-main.c | 6 +- configure.in | 4 +- dbus/dbus-connection.c | 29 ++--- dbus/dbus-memory.c | 178 ++++++++++++++++++++++++++++++ dbus/dbus-message.c | 79 ++++++++++---- dbus/dbus-message.h | 2 +- dbus/dbus-server-debug.c | 28 +++-- dbus/dbus-server.c | 6 +- dbus/dbus-timeout.c | 5 +- dbus/dbus-timeout.h | 1 - dbus/dbus-transport-debug.c | 259 +++++++++++++++++++++++--------------------- dbus/dbus-transport-unix.c | 7 +- glib/dbus-gmain.c | 2 +- 18 files changed, 514 insertions(+), 203 deletions(-) diff --git a/ChangeLog b/ChangeLog index 296d334..b2f8e07 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2003-03-14 Havoc Pennington + + * dbus/dbus-memory.c: add a "detect buffer overwrites on free" + cheesy hack + + * dbus/dbus-transport-debug.c: rework this a good bit to be + less complicated. hopefully still works. + + * dbus/dbus-server-debug.c (handle_new_client): remove timeout + manually + + * glib/dbus-gmain.c (timeout_handler): don't remove timeout + after running it + + * dbus/dbus-message.c (dbus_message_copy): rename from + dbus_message_new_from_message, fix it up to copy + all the message fields, add test case + + * bus/dispatch.c (bus_dispatch_test): add some more test code, + not quite passing yet + 2003-03-14 Havoc Pennington * bus/loop.c (bus_loop_iterate): add this so we can "run loop diff --git a/bus/bus.c b/bus/bus.c index e376ae4..b717cac 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -46,7 +46,7 @@ server_watch_callback (DBusWatch *watch, { BusContext *context = data; - dbus_server_handle_watch (context->server, watch, condition); + dbus_server_handle_watch (context->server, watch, condition); } static dbus_bool_t @@ -64,6 +64,7 @@ remove_server_watch (DBusWatch *watch, bus_loop_remove_watch (watch, server_watch_callback, context); } + static void server_timeout_callback (DBusTimeout *timeout, void *data) @@ -95,7 +96,7 @@ new_connection_callback (DBusServer *server, if (!bus_connections_setup_connection (context->connections, new_connection)) _dbus_verbose ("No memory to setup new connection\n"); - /* on OOM, we won't have ref'd the connection so it will die */ + /* on OOM, we won't have ref'd the connection so it will die. */ } BusContext* diff --git a/bus/connection.c b/bus/connection.c index f046339..700ca46 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -143,7 +143,8 @@ connection_watch_callback (DBusWatch *watch, dbus_connection_handle_watch (connection, watch, condition); - while (dbus_connection_dispatch_message (connection)); + while (dbus_connection_dispatch_message (connection)) + ; dbus_connection_unref (connection); } @@ -166,7 +167,15 @@ static void connection_timeout_callback (DBusTimeout *timeout, void *data) { + DBusConnection *connection = data; + + dbus_connection_ref (connection); + dbus_timeout_handle (timeout); + + while (dbus_connection_dispatch_message (connection)) + ; + dbus_connection_unref (connection); } static dbus_bool_t diff --git a/bus/dispatch.c b/bus/dispatch.c index 04d68ec..928e438 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -28,6 +28,7 @@ #include "utils.h" #include "bus.h" #include "test.h" +#include "loop.h" #include #include @@ -172,12 +173,15 @@ bus_dispatch (DBusConnection *connection, _dbus_assert (message_name != NULL); /* DBusMessageLoader is supposed to check this */ + _dbus_verbose ("DISPATCH: %s to %s\n", + message_name, service_name ? service_name : "peer"); + /* If service_name is NULL, this is a message to the bus daemon, not intended * to actually go "on the bus"; e.g. a peer-to-peer ping. Handle these * immediately, especially disconnection messages. */ if (service_name == NULL) - { + { if (strcmp (message_name, DBUS_MESSAGE_LOCAL_DISCONNECT) == 0) bus_connection_disconnected (connection); @@ -374,12 +378,15 @@ bus_dispatch_remove_connection (DBusConnection *connection) #ifdef DBUS_BUILD_TESTS static void -run_test_bus (BusContext *context) +flush_bus (BusContext *context) { - - + while (bus_loop_iterate (FALSE)) + ; } +/* returns TRUE if the correct thing happens, + * but the correct thing may include OOM errors. + */ static dbus_bool_t check_hello_message (BusContext *context, DBusConnection *connection) @@ -391,11 +398,28 @@ check_hello_message (BusContext *context, DBUS_MESSAGE_HELLO); if (message == NULL) - _dbus_assert_not_reached ("no memory"); + return TRUE; if (!dbus_connection_send (connection, message, &serial)) - _dbus_assert_not_reached ("no memory"); + return TRUE; + + dbus_message_unref (message); + + flush_bus (context); + message = dbus_connection_pop_message (connection); + if (message == NULL) + { + _dbus_warn ("Did not receive a reply to %s %d on %p\n", + DBUS_MESSAGE_HELLO, serial, connection); + return FALSE; + } + + _dbus_verbose ("Received %s on %p\n", + dbus_message_get_name (message), connection); + + dbus_message_unref (message); + return TRUE; } @@ -410,8 +434,6 @@ bus_dispatch_test (const DBusString *test_data_dir) DBusConnection *baz; DBusResultCode result; - return TRUE; /* FIXME */ - dbus_error_init (&error); context = bus_context_new ("debug:name=test-server", activation_dirs, @@ -431,7 +453,12 @@ bus_dispatch_test (const DBusString *test_data_dir) if (baz == NULL) _dbus_assert_not_reached ("could not alloc connection"); - + if (!check_hello_message (context, foo)) + _dbus_assert_not_reached ("hello message failed"); + if (!check_hello_message (context, bar)) + _dbus_assert_not_reached ("hello message failed"); + if (!check_hello_message (context, baz)) + _dbus_assert_not_reached ("hello message failed"); return TRUE; } diff --git a/bus/loop.c b/bus/loop.c index 72bf99e..1061474 100644 --- a/bus/loop.c +++ b/bus/loop.c @@ -262,8 +262,8 @@ bus_loop_remove_timeout (DBusTimeout *timeout, timeout, function, data); } -/* Returns TRUE if we dispatch any callbacks, which is just used in - * test code as a debug hack +/* Returns TRUE if we have any timeouts or ready file descriptors, + * which is just used in test code as a debug hack */ dbus_bool_t @@ -283,7 +283,12 @@ bus_loop_iterate (dbus_bool_t block) fds = NULL; watches_for_fds = NULL; - + +#if 0 + _dbus_verbose (" iterate %d timeouts %d watches\n", + timeout_count, watch_count); +#endif + if (callbacks == NULL) { bus_loop_quit (); @@ -343,7 +348,9 @@ bus_loop_iterate (dbus_bool_t block) { unsigned long tv_sec; unsigned long tv_usec; - + + retval = TRUE; + _dbus_get_current_time (&tv_sec, &tv_usec); link = _dbus_list_get_first_link (&callbacks); @@ -445,16 +452,23 @@ bus_loop_iterate (dbus_bool_t block) (tv_sec - tcb->last_tv_sec) * 1000 + (tv_usec - tcb->last_tv_usec) / 1000; +#if 0 + _dbus_verbose (" interval = %lu elapsed = %lu\n", + interval, elapsed); +#endif + if (interval <= elapsed) { /* Save last callback time and fire this timeout */ tcb->last_tv_sec = tv_sec; tcb->last_tv_usec = tv_usec; - + +#if 0 + _dbus_verbose (" invoking timeout\n"); +#endif + (* tcb->function) (tcb->timeout, cb->data); - - retval = TRUE; } } @@ -504,6 +518,7 @@ bus_loop_iterate (dbus_bool_t block) (* wcb->function) (wcb->watch, condition, ((Callback*)wcb)->data); + retval = TRUE; } } diff --git a/bus/test-main.c b/bus/test-main.c index 503d996..26e9110 100644 --- a/bus/test-main.c +++ b/bus/test-main.c @@ -53,10 +53,14 @@ main (int argc, char **argv) if (!bus_dispatch_test (&test_data_dir)) die ("dispatch"); + + printf ("Success\n"); return 0; #else /* DBUS_BUILD_TESTS */ - + + printf ("Not compiled with test support\n"); + return 0; #endif } diff --git a/configure.in b/configure.in index 0bf239a..3e5e57f 100644 --- a/configure.in +++ b/configure.in @@ -330,13 +330,13 @@ echo " " if test x$enable_tests = xyes; then - echo "NOTE: building with unit tests increases the size of the installed library" + echo "NOTE: building with unit tests increases the size of the installed library and renders it insecure" fi if test x$enable_gcov = xyes; then echo "NOTE: building with coverage profiling is definitely for developers only" fi if test x$enable_verbose_mode = xyes; then - echo "NOTE: building with verbose mode increases library size, but is probably a good idea anyway." + echo "NOTE: building with verbose mode increases library size and may slightly increase security risk, but aids debugging." fi if test x$enable_asserts = xyes; then echo "NOTE: building with assertions increases library size, but is probably a good idea anyway." diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index d52341a..c5df561 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -207,8 +207,9 @@ _dbus_connection_queue_received_message (DBusConnection *connection, _dbus_connection_wakeup_mainloop (connection); _dbus_assert (dbus_message_get_name (message) != NULL); - _dbus_verbose ("Incoming message %p (%s) added to queue, %d incoming\n", + _dbus_verbose ("Message %p (%s) added to incoming queue %p, %d incoming\n", message, dbus_message_get_name (message), + connection, connection->n_incoming); return TRUE; @@ -234,8 +235,8 @@ _dbus_connection_queue_synthesized_message_link (DBusConnection *connection, _dbus_connection_wakeup_mainloop (connection); - _dbus_verbose ("Incoming synthesized message %p added to queue, %d incoming\n", - link->data, connection->n_incoming); + _dbus_verbose ("Synthesized message %p added to incoming queue %p, %d incoming\n", + link->data, connection, connection->n_incoming); } @@ -283,8 +284,8 @@ _dbus_connection_message_sent (DBusConnection *connection, connection->n_outgoing -= 1; - _dbus_verbose ("Message %p removed from outgoing queue, %d left to send\n", - message, connection->n_outgoing); + _dbus_verbose ("Message %p removed from outgoing queue %p, %d left to send\n", + message, connection, connection->n_outgoing); if (connection->n_outgoing == 0) _dbus_transport_messages_pending (connection->transport, @@ -334,7 +335,7 @@ _dbus_connection_remove_watch (DBusConnection *connection, * available. Otherwise records the timeout to be added when said * function is available. Also re-adds the timeout if the * DBusAddTimeoutFunction changes. May fail due to lack of memory. - * The timeout will fire only one time. + * The timeout will fire repeatedly until removed. * * @param connection the connection. * @param timeout the timeout to add. @@ -1035,9 +1036,10 @@ dbus_connection_send_preallocated (DBusConnection *connection, dbus_message_ref (message); connection->n_outgoing += 1; - _dbus_verbose ("Message %p (%s) added to outgoing queue, %d pending to send\n", + _dbus_verbose ("Message %p (%s) added to outgoing queue %p, %d pending to send\n", message, dbus_message_get_name (message), + connection, connection->n_outgoing); if (dbus_message_get_serial (message) == -1) @@ -1558,8 +1560,8 @@ _dbus_connection_pop_message_unlocked (DBusConnection *connection) message = _dbus_list_pop_first (&connection->incoming_messages); connection->n_incoming -= 1; - _dbus_verbose ("Incoming message %p removed from queue, %d incoming\n", - message, connection->n_incoming); + _dbus_verbose ("Message %p removed from incoming queue %p, %d incoming\n", + message, connection, connection->n_incoming); return message; } @@ -1859,11 +1861,10 @@ dbus_connection_set_watch_functions (DBusConnection *connection, * g_timeout_add. * * The DBusTimeout can be queried for the timer interval using - * dbus_timeout_get_interval. - * - * Once a timeout occurs, dbus_timeout_handle should be called to invoke - * the timeout's callback, and the timeout should be automatically - * removed. i.e. timeouts are one-shot. + * dbus_timeout_get_interval(). dbus_timeout_handle() should + * be called repeatedly, each time the interval elapses, starting + * after it has elapsed once. The timeout stops firing when + * it is removed with the given remove_function. * * @param connection the connection. * @param add_function function to add a timeout. diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c index 14ce23d..5f0d9b8 100644 --- a/dbus/dbus-memory.c +++ b/dbus/dbus-memory.c @@ -76,6 +76,13 @@ static dbus_bool_t inited = FALSE; static int fail_counts = -1; static size_t fail_size = 0; +static dbus_bool_t guards = FALSE; +#define GUARD_VALUE 0xdeadbeef +#define GUARD_INFO_SIZE 8 +#define GUARD_START_PAD 16 +#define GUARD_END_PAD 16 +#define GUARD_START_OFFSET (GUARD_START_PAD + GUARD_INFO_SIZE) +#define GUARD_EXTRA_SIZE (GUARD_START_OFFSET + GUARD_END_PAD) #endif #ifdef DBUS_BUILD_TESTS @@ -92,10 +99,133 @@ initialize_malloc_debug (void) if (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN") != NULL) fail_size = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN")); + + if (_dbus_getenv ("DBUS_MALLOC_GUARDS") != NULL) + guards = TRUE; inited = TRUE; } } + +typedef enum +{ + SOURCE_UNKNOWN, + SOURCE_MALLOC, + SOURCE_REALLOC, + SOURCE_MALLOC_ZERO, + SOURCE_REALLOC_NULL +} BlockSource; + +static const char* +source_string (BlockSource source) +{ + switch (source) + { + case SOURCE_UNKNOWN: + return "unknown"; + case SOURCE_MALLOC: + return "malloc"; + case SOURCE_REALLOC: + return "realloc"; + case SOURCE_MALLOC_ZERO: + return "malloc0"; + case SOURCE_REALLOC_NULL: + return "realloc(NULL)"; + } + _dbus_assert_not_reached ("Invalid malloc block source ID"); + return "invalid!"; +} + +static void +check_guards (void *free_block) +{ + if (free_block != NULL) + { + unsigned char *block = ((unsigned char*)free_block) - GUARD_START_OFFSET; + size_t requested_bytes = *(dbus_uint32_t*)block; + BlockSource source = *(dbus_uint32_t*)(block + 4); + unsigned int i; + dbus_bool_t failed; + + failed = FALSE; + +#if 0 + _dbus_verbose ("Checking %d bytes request from source %s\n", + requested_bytes, source_string (source)); +#endif + + i = GUARD_INFO_SIZE; + while (i < GUARD_START_OFFSET) + { + dbus_uint32_t value = *(dbus_uint32_t*) &block[i]; + if (value != GUARD_VALUE) + { + _dbus_warn ("Block of %u bytes from %s had start guard value 0x%x at %d expected 0x%x\n", + requested_bytes, source_string (source), + value, i, GUARD_VALUE); + failed = TRUE; + } + + i += 4; + } + + i = GUARD_START_OFFSET + requested_bytes; + while (i < (GUARD_START_OFFSET + requested_bytes + GUARD_END_PAD)) + { + dbus_uint32_t value = *(dbus_uint32_t*) &block[i]; + if (value != GUARD_VALUE) + { + _dbus_warn ("Block of %u bytes from %s had end guard value 0x%x at %d expected 0x%x\n", + requested_bytes, source_string (source), + value, i, GUARD_VALUE); + failed = TRUE; + } + + i += 4; + } + + if (failed) + _dbus_assert_not_reached ("guard value corruption"); + } +} + +static void* +set_guards (void *real_block, + size_t requested_bytes, + BlockSource source) +{ + unsigned char *block = real_block; + unsigned int i; + + if (block == NULL) + return NULL; + + _dbus_assert (GUARD_START_OFFSET + GUARD_END_PAD == GUARD_EXTRA_SIZE); + + *((dbus_uint32_t*)block) = requested_bytes; + *((dbus_uint32_t*)(block + 4)) = source; + + i = GUARD_INFO_SIZE; + while (i < GUARD_START_OFFSET) + { + (*(dbus_uint32_t*) &block[i]) = GUARD_VALUE; + + i += 4; + } + + i = GUARD_START_OFFSET + requested_bytes; + while (i < (GUARD_START_OFFSET + requested_bytes + GUARD_END_PAD)) + { + (*(dbus_uint32_t*) &block[i]) = GUARD_VALUE; + + i += 4; + } + + check_guards (block + GUARD_START_OFFSET); + + return block + GUARD_START_OFFSET; +} + #endif /** @@ -127,6 +257,13 @@ dbus_malloc (size_t bytes) #if DBUS_BUILD_TESTS else if (fail_size != 0 && bytes > fail_size) return NULL; + else if (guards) + { + void *block; + + block = malloc (bytes + GUARD_EXTRA_SIZE); + return set_guards (block, bytes, SOURCE_MALLOC); + } #endif else return malloc (bytes); @@ -161,6 +298,13 @@ dbus_malloc0 (size_t bytes) #if DBUS_BUILD_TESTS else if (fail_size != 0 && bytes > fail_size) return NULL; + else if (guards) + { + void *block; + + block = calloc (bytes + GUARD_EXTRA_SIZE, 1); + return set_guards (block, bytes, SOURCE_MALLOC_ZERO); + } #endif else return calloc (bytes, 1); @@ -200,6 +344,30 @@ dbus_realloc (void *memory, #if DBUS_BUILD_TESTS else if (fail_size != 0 && bytes > fail_size) return NULL; + else if (guards) + { + if (memory) + { + void *block; + + check_guards (memory); + + block = realloc (((unsigned char*)memory) - GUARD_START_OFFSET, + bytes + GUARD_EXTRA_SIZE); + + /* old guards shouldn't have moved */ + check_guards (((unsigned char*)block) + GUARD_START_OFFSET); + + return set_guards (block, bytes, SOURCE_REALLOC); + } + else + { + void *block; + + block = malloc (bytes + GUARD_EXTRA_SIZE); + return set_guards (block, bytes, SOURCE_REALLOC_NULL); + } + } #endif else { @@ -216,6 +384,16 @@ dbus_realloc (void *memory, void dbus_free (void *memory) { +#ifdef DBUS_BUILD_TESTS + if (guards) + { + check_guards (memory); + if (memory) + free (((unsigned char*)memory) - GUARD_START_OFFSET); + return; + } +#endif + if (memory) /* we guarantee it's safe to free (NULL) */ free (memory); } diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 6a3c661..b1fc648 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -897,7 +897,7 @@ dbus_message_new_error_reply (DBusMessage *original_message, * @returns the new message. */ DBusMessage * -dbus_message_new_from_message (const DBusMessage *message) +dbus_message_copy (const DBusMessage *message) { DBusMessage *retval; int i; @@ -908,7 +908,11 @@ dbus_message_new_from_message (const DBusMessage *message) retval->refcount = 1; retval->byte_order = message->byte_order; - + retval->client_serial = message->client_serial; + retval->reply_serial = message->reply_serial; + retval->header_padding = message->header_padding; + retval->locked = FALSE; + if (!_dbus_string_init (&retval->header, _DBUS_INT_MAX)) { dbus_free (retval); @@ -3590,6 +3594,37 @@ dbus_internal_do_not_use_foreach_message_file (const char *test_d return retval; } +static void +verify_test_message (DBusMessage *message) +{ + dbus_int32_t our_int; + char *our_str; + double our_double; + dbus_bool_t our_bool; + + if (!dbus_message_get_args (message, NULL, + DBUS_TYPE_INT32, &our_int, + DBUS_TYPE_STRING, &our_str, + DBUS_TYPE_DOUBLE, &our_double, + DBUS_TYPE_BOOLEAN, &our_bool, + 0)) + _dbus_assert_not_reached ("Could not get arguments"); + + if (our_int != -0x12345678) + _dbus_assert_not_reached ("integers differ!"); + + if (our_double != 3.14159) + _dbus_assert_not_reached ("doubles differ!"); + + if (strcmp (our_str, "Test string") != 0) + _dbus_assert_not_reached ("strings differ!"); + + if (!our_bool) + _dbus_assert_not_reached ("booleans differ"); + + dbus_free (our_str); +} + /** * @ingroup DBusMessageInternals * Unit test for DBusMessage. @@ -3603,10 +3638,9 @@ _dbus_message_test (const char *test_data_dir) DBusMessageLoader *loader; int i; const char *data; - dbus_int32_t our_int; - char *our_str; - double our_double; - dbus_bool_t our_bool; + DBusMessage *copy; + const char *name1; + const char *name2; /* Test the vararg functions */ message = dbus_message_new ("org.freedesktop.DBus.Test", "testMessage"); @@ -3622,28 +3656,29 @@ _dbus_message_test (const char *test_data_dir) _dbus_verbose_bytes_of_string (&message->body, 0, _dbus_string_get_length (&message->body)); - if (!dbus_message_get_args (message, NULL, - DBUS_TYPE_INT32, &our_int, - DBUS_TYPE_STRING, &our_str, - DBUS_TYPE_DOUBLE, &our_double, - DBUS_TYPE_BOOLEAN, &our_bool, - 0)) - _dbus_assert_not_reached ("Could not get arguments"); + verify_test_message (message); - if (our_int != -0x12345678) - _dbus_assert_not_reached ("integers differ!"); + copy = dbus_message_copy (message); + + _dbus_assert (message->client_serial == copy->client_serial); + _dbus_assert (message->reply_serial == copy->reply_serial); + _dbus_assert (message->header_padding == copy->header_padding); + + _dbus_assert (_dbus_string_get_length (&message->header) == + _dbus_string_get_length (©->header)); - if (our_double != 3.14159) - _dbus_assert_not_reached ("doubles differ!"); + _dbus_assert (_dbus_string_get_length (&message->body) == + _dbus_string_get_length (©->body)); - if (strcmp (our_str, "Test string") != 0) - _dbus_assert_not_reached ("strings differ!"); + verify_test_message (copy); - if (!our_bool) - _dbus_assert_not_reached ("booleans differ"); + name1 = dbus_message_get_name (message); + name2 = dbus_message_get_name (copy); + + _dbus_assert (strcmp (name1, name2) == 0); - dbus_free (our_str); dbus_message_unref (message); + dbus_message_unref (copy); message = dbus_message_new ("org.freedesktop.DBus.Test", "testMessage"); _dbus_message_set_serial (message, 1); diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index d30a0a3..1d5bbeb 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -43,7 +43,7 @@ DBusMessage* dbus_message_new_reply (DBusMessage *original_message) DBusMessage* dbus_message_new_error_reply (DBusMessage *original_message, const char *error_name, const char *error_message); -DBusMessage *dbus_message_new_from_message (const DBusMessage *message); +DBusMessage *dbus_message_copy (const DBusMessage *message); void dbus_message_ref (DBusMessage *message); void dbus_message_unref (DBusMessage *message); diff --git a/dbus/dbus-server-debug.c b/dbus/dbus-server-debug.c index 5f79e81..b1318e3 100644 --- a/dbus/dbus-server-debug.c +++ b/dbus/dbus-server-debug.c @@ -181,7 +181,7 @@ typedef struct { DBusServer *server; DBusTransport *transport; - + DBusTimeout *timeout; } ServerAndTransport; static void @@ -190,12 +190,13 @@ handle_new_client (void *data) ServerAndTransport *st = data; DBusTransport *transport; DBusConnection *connection; + + _dbus_verbose (" new debug client transport %p connecting to server\n", + st->transport); transport = _dbus_transport_debug_server_new (st->transport); if (transport == NULL) - { - return; - } + return; connection = _dbus_connection_new_for_transport (transport); _dbus_transport_unref (transport); @@ -214,9 +215,14 @@ handle_new_client (void *data) st->server->new_connection_data); dbus_server_unref (st->server); } + + _dbus_server_remove_timeout (st->server, st->timeout); /* If no one grabbed a reference, the connection will die. */ dbus_connection_unref (connection); + + /* killing timeout frees both "st" and "timeout" */ + _dbus_timeout_unref (st->timeout); } /** @@ -231,7 +237,6 @@ dbus_bool_t _dbus_server_debug_accept_transport (DBusServer *server, DBusTransport *transport) { - DBusTimeout *timeout = NULL; ServerAndTransport *st = NULL; st = dbus_new (ServerAndTransport, 1); @@ -241,22 +246,21 @@ _dbus_server_debug_accept_transport (DBusServer *server, st->transport = transport; st->server = server; - timeout = _dbus_timeout_new (DEFAULT_INTERVAL, handle_new_client, st, dbus_free); + st->timeout = _dbus_timeout_new (DEFAULT_INTERVAL, handle_new_client, st, + dbus_free); - if (timeout == NULL) + if (st->timeout == NULL) goto failed; - if (!_dbus_server_add_timeout (server, timeout)) + if (!_dbus_server_add_timeout (server, st->timeout)) goto failed; - - _dbus_timeout_unref (timeout); return TRUE; failed: + if (st->timeout) + _dbus_timeout_unref (st->timeout); dbus_free (st); - if (timeout) - _dbus_timeout_unref (timeout); return FALSE; } diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index cfcc0dd..a5e5e2b 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -145,8 +145,10 @@ _dbus_server_remove_watch (DBusServer *server, } /** - * Adds a timeout for this server, chaining out to application-provided - * timeout handlers. The timeout will fire only one time. + * Adds a timeout for this server, chaining out to + * application-provided timeout handlers. The timeout should be + * repeatedly handled with dbus_timeout_handle() at its given interval + * until it is removed. * * @param server the server. * @param timeout the timeout to add. diff --git a/dbus/dbus-timeout.c b/dbus/dbus-timeout.c index 408de42..09e54b3 100644 --- a/dbus/dbus-timeout.c +++ b/dbus/dbus-timeout.c @@ -319,7 +319,10 @@ _dbus_timeout_list_remove_timeout (DBusTimeoutList *timeout_list, */ /** - * Gets the timeout interval. + * Gets the timeout interval. The dbus_timeout_handle() + * should be called each time this interval elapses, + * starting after it elapses once. + * * @param timeout the DBusTimeout object. * @returns the interval in milliseconds. */ diff --git a/dbus/dbus-timeout.h b/dbus/dbus-timeout.h index 9de12e8..2d7112a 100644 --- a/dbus/dbus-timeout.h +++ b/dbus/dbus-timeout.h @@ -41,7 +41,6 @@ DBusTimeout* _dbus_timeout_new (int interval, void _dbus_timeout_ref (DBusTimeout *timeout); void _dbus_timeout_unref (DBusTimeout *timeout); - DBusTimeoutList *_dbus_timeout_list_new (void); void _dbus_timeout_list_free (DBusTimeoutList *timeout_list); dbus_bool_t _dbus_timeout_list_set_functions (DBusTimeoutList *timeout_list, diff --git a/dbus/dbus-transport-debug.c b/dbus/dbus-transport-debug.c index cb4b3c2..42d1efd 100644 --- a/dbus/dbus-transport-debug.c +++ b/dbus/dbus-transport-debug.c @@ -2,6 +2,7 @@ /* dbus-transport-debug.c In-proc debug subclass of DBusTransport * * Copyright (C) 2003 CodeFactory AB + * Copyright (C) 2003 Red Hat, Inc. * * Licensed under the Academic Free License version 1.2 * @@ -47,6 +48,13 @@ #define DEFAULT_INTERVAL 1 /** + * Hack due to lack of OOM handling in a couple places + */ +#define WAIT_FOR_MEMORY() _dbus_sleep_milliseconds (250) + +static void check_timeout (DBusTransport *transport); + +/** * Opaque object representing a debug transport. * */ @@ -59,96 +67,21 @@ struct DBusTransportDebug { DBusTransport base; /**< Parent instance */ - DBusTimeout *write_timeout; /**< Timeout for reading. */ - DBusTimeout *read_timeout; /**< Timeout for writing. */ + DBusTimeout *timeout; /**< Timeout for moving messages. */ DBusTransport *other_end; /**< The transport that this transport is connected to. */ -}; - -static void -debug_finalize (DBusTransport *transport) -{ - _dbus_transport_finalize_base (transport); - dbus_free (transport); -} - -static void -do_reading (DBusTransport *transport) -{ - DBusTransportDebug *debug_transport = (DBusTransportDebug*) transport; - - if (transport->disconnected) - return; - - /* Now dispatch the messages */ - if (dbus_connection_dispatch_message (transport->connection)) - { - debug_transport->read_timeout = - _dbus_timeout_new (DEFAULT_INTERVAL, (DBusTimeoutHandler)do_reading, - transport, NULL); - if (!_dbus_connection_add_timeout (transport->connection, - debug_transport->read_timeout)) - { - _dbus_timeout_unref (debug_transport->read_timeout); - debug_transport->read_timeout = NULL; - } - } -} + unsigned int timeout_added : 1; /**< Whether timeout has been added */ +}; -static void -check_read_timeout (DBusTransport *transport) +/* move messages in both directions */ +static dbus_bool_t +move_messages (DBusTransport *transport) { DBusTransportDebug *debug_transport = (DBusTransportDebug*) transport; - dbus_bool_t need_read_timeout; - - if (transport->connection == NULL) - return; - - _dbus_transport_ref (transport); - - need_read_timeout = dbus_connection_get_n_messages (transport->connection) > 0; if (transport->disconnected) - need_read_timeout = FALSE; - - if (need_read_timeout && - debug_transport->read_timeout == NULL) - { - debug_transport->read_timeout = - _dbus_timeout_new (DEFAULT_INTERVAL, (DBusTimeoutHandler)do_reading, - transport, NULL); - - if (debug_transport->read_timeout == NULL) - goto out; - - if (!_dbus_connection_add_timeout (transport->connection, - debug_transport->read_timeout)) - { - _dbus_timeout_unref (debug_transport->read_timeout); - debug_transport->read_timeout = NULL; - - goto out; - } - } - else if (!need_read_timeout && - debug_transport->read_timeout != NULL) - { - _dbus_connection_remove_timeout (transport->connection, - debug_transport->read_timeout); - _dbus_timeout_unref (debug_transport->read_timeout); - debug_transport->read_timeout = NULL; - } - - out: - _dbus_transport_unref (transport); -} - -static void -do_writing (DBusTransport *transport) -{ - if (transport->disconnected) - return; + return TRUE; while (!transport->disconnected && _dbus_connection_have_messages_to_send (transport->connection)) @@ -156,65 +89,106 @@ do_writing (DBusTransport *transport) DBusMessage *message, *copy; message = _dbus_connection_get_message_to_send (transport->connection); - _dbus_message_lock (message); + _dbus_assert (message != NULL); - copy = dbus_message_new_from_message (message); + copy = dbus_message_copy (message); + if (copy == NULL) + return FALSE; + + _dbus_message_lock (message); _dbus_connection_message_sent (transport->connection, message); + + _dbus_verbose (" -->transporting message %s from %s %p to %s %p\n", + dbus_message_get_name (copy), + transport->is_server ? "server" : "client", + transport->connection, + debug_transport->other_end->is_server ? "server" : "client", + debug_transport->other_end->connection); - _dbus_connection_queue_received_message (((DBusTransportDebug *)transport)->other_end->connection, + _dbus_connection_queue_received_message (debug_transport->other_end->connection, copy); dbus_message_unref (copy); } - check_read_timeout (((DBusTransportDebug *)transport)->other_end); + if (debug_transport->other_end && + !debug_transport->other_end->disconnected && + _dbus_connection_have_messages_to_send (debug_transport->other_end->connection)) + { + if (!move_messages (debug_transport->other_end)) + return FALSE; + } + + return TRUE; } static void -check_write_timeout (DBusTransport *transport) +timeout_handler (void *data) { - DBusTransportDebug *debug_transport = (DBusTransportDebug *)transport; - dbus_bool_t need_write_timeout; + DBusTransport *transport = data; - if (transport->connection == NULL) - return; + while (!move_messages (transport)) + WAIT_FOR_MEMORY (); - _dbus_transport_ref (transport); + check_timeout (transport); +} - need_write_timeout = transport->messages_need_sending; +static void +check_timeout (DBusTransport *transport) +{ + DBusTransportDebug *debug_transport = (DBusTransportDebug*) transport; - if (transport->disconnected) - need_write_timeout = FALSE; - - if (need_write_timeout && - debug_transport->write_timeout == NULL) + if (transport->connection && + transport->authenticated && + (transport->messages_need_sending || + (debug_transport->other_end && + debug_transport->other_end->messages_need_sending))) { - debug_transport->write_timeout = - _dbus_timeout_new (DEFAULT_INTERVAL, (DBusTimeoutHandler)do_writing, - transport, NULL); - - if (debug_transport->write_timeout == NULL) - goto out; - - if (!_dbus_connection_add_timeout (transport->connection, - debug_transport->write_timeout)) - { - _dbus_timeout_unref (debug_transport->write_timeout); - debug_transport->write_timeout = NULL; - } + if (!debug_transport->timeout_added) + { + /* FIXME; messages_pending is going to have to + * handle OOM somehow (probably being part of + * PreallocatedSend). See also dbus-transport-unix.c + * check_write_watch() + */ + while (!_dbus_connection_add_timeout (transport->connection, + debug_transport->timeout)) + WAIT_FOR_MEMORY (); + debug_transport->timeout_added = TRUE; + } } - else if (!need_write_timeout && - debug_transport->write_timeout != NULL) + else { - _dbus_connection_remove_timeout (transport->connection, - debug_transport->write_timeout); - _dbus_timeout_unref (debug_transport->write_timeout); - debug_transport->write_timeout = NULL; + if (debug_transport->timeout_added) + { + _dbus_connection_remove_timeout (transport->connection, + debug_transport->timeout); + debug_transport->timeout_added = FALSE; + } } +} - out: - _dbus_transport_unref (transport); +static void +debug_finalize (DBusTransport *transport) +{ + DBusTransportDebug *debug_transport = (DBusTransportDebug*) transport; + + if (debug_transport->timeout_added) + _dbus_connection_remove_timeout (transport->connection, + debug_transport->timeout); + + if (debug_transport->other_end) + { + _dbus_transport_disconnect (debug_transport->other_end); + debug_transport->other_end = NULL; + } + + _dbus_transport_finalize_base (transport); + + _dbus_timeout_unref (debug_transport->timeout); + + dbus_free (transport); } static void @@ -232,13 +206,14 @@ debug_disconnect (DBusTransport *transport) static void debug_connection_set (DBusTransport *transport) { + check_timeout (transport); } static void debug_messages_pending (DBusTransport *transport, int messages_pending) { - check_write_timeout (transport); + check_timeout (transport); } static void @@ -246,6 +221,7 @@ debug_do_iteration (DBusTransport *transport, unsigned int flags, int timeout_milliseconds) { + move_messages (transport); } static void @@ -263,6 +239,16 @@ static DBusTransportVTable debug_vtable = { debug_live_messages_changed }; +static dbus_bool_t +create_timeout_object (DBusTransportDebug *debug_transport) +{ + debug_transport->timeout = _dbus_timeout_new (DEFAULT_INTERVAL, + timeout_handler, + debug_transport, NULL); + + return debug_transport->timeout != NULL; +} + /** * Creates a new debug server transport. * @@ -288,11 +274,21 @@ _dbus_transport_debug_server_new (DBusTransport *client) return NULL; } + if (!create_timeout_object (debug_transport)) + { + _dbus_transport_finalize_base (&debug_transport->base); + dbus_free (debug_transport); + return NULL; + } + debug_transport->base.authenticated = TRUE; /* Connect the two transports */ debug_transport->other_end = client; ((DBusTransportDebug *)client)->other_end = (DBusTransport *)debug_transport; + + _dbus_verbose (" new debug server transport %p created, other end %p\n", + debug_transport, debug_transport->other_end); return (DBusTransport *)debug_transport; } @@ -335,19 +331,30 @@ _dbus_transport_debug_client_new (const char *server_name, dbus_set_result (result, DBUS_RESULT_NO_MEMORY); return NULL; } - - if (!_dbus_server_debug_accept_transport (debug_server, (DBusTransport *)debug_transport)) + + if (!create_timeout_object (debug_transport)) { _dbus_transport_finalize_base (&debug_transport->base); - - dbus_free (debug_transport); - dbus_set_result (result, DBUS_RESULT_IO_ERROR); + dbus_free (debug_transport); + dbus_set_result (result, DBUS_RESULT_NO_MEMORY); + return NULL; + } + + if (!_dbus_server_debug_accept_transport (debug_server, + (DBusTransport *)debug_transport)) + { + _dbus_timeout_unref (debug_transport->timeout); + _dbus_transport_finalize_base (&debug_transport->base); + dbus_free (debug_transport); + dbus_set_result (result, DBUS_RESULT_NO_MEMORY); return NULL; - } /* FIXME: Prolly wrong to do this. */ debug_transport->base.authenticated = TRUE; + + _dbus_verbose (" new debug client transport %p created, other end %p\n", + debug_transport, debug_transport->other_end); return (DBusTransport *)debug_transport; } diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index 81c18b4..a2b8a38 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -136,7 +136,12 @@ check_write_watch (DBusTransport *transport) _dbus_watch_new (unix_transport->fd, DBUS_WATCH_WRITABLE); - /* we can maybe add it some other time, just silently bomb */ + /* FIXME this is total crack. The proper fix is probably to + * allocate the write watch on transport creation, keep it + * allocated. But that doesn't solve needing memory to add the + * watch. messages_pending is going to have to handle OOM + * somehow (probably being part of PreallocatedSend) + */ if (unix_transport->write_watch == NULL) goto out; diff --git a/glib/dbus-gmain.c b/glib/dbus-gmain.c index b0ec7c4..6ce1388 100644 --- a/glib/dbus-gmain.c +++ b/glib/dbus-gmain.c @@ -284,7 +284,7 @@ timeout_handler (gpointer data) dbus_timeout_handle (timeout); - return FALSE; + return TRUE; } static dbus_bool_t -- 2.7.4