From 4c793cfc76360676b3881d7943b49adf892594d2 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 13:22:05 +0200 Subject: libpulse: Store pa_stream pointers to hashmaps instead of dynarrays. Since the stream identifiers (channels) are monotonically growing integer, it isn't a good idea to use them as index to a dynamic array, because the array will grow all the time. This is not a problem with client connections that don't create many streams, but, for example, long-running clients that use libcanberra for playing event sounds, this means that the client connection effectively leaks memory. --- src/pulse/context.c | 12 ++++++------ src/pulse/internal.h | 3 +-- src/pulse/stream.c | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/pulse/context.c b/src/pulse/context.c index e33143d9..00184920 100644 --- a/src/pulse/context.c +++ b/src/pulse/context.c @@ -63,7 +63,7 @@ #include #include #include -#include +#include #include #include #include @@ -160,8 +160,8 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char * c->client = NULL; c->pstream = NULL; c->pdispatch = NULL; - c->playback_streams = pa_dynarray_new(); - c->record_streams = pa_dynarray_new(); + c->playback_streams = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); + c->record_streams = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); c->client_index = PA_INVALID_INDEX; c->use_rtclock = pa_mainloop_is_our_api(mainloop); @@ -266,9 +266,9 @@ static void context_free(pa_context *c) { #endif if (c->record_streams) - pa_dynarray_free(c->record_streams, NULL, NULL); + pa_hashmap_free(c->record_streams, NULL, NULL); if (c->playback_streams) - pa_dynarray_free(c->playback_streams, NULL, NULL); + pa_hashmap_free(c->playback_streams, NULL, NULL); if (c->mempool) pa_mempool_free(c->mempool); @@ -375,7 +375,7 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o pa_context_ref(c); - if ((s = pa_dynarray_get(c->record_streams, channel))) { + if ((s = pa_hashmap_get(c->record_streams, PA_UINT32_TO_PTR(channel)))) { if (chunk->memblock) { pa_memblockq_seek(s->record_memblockq, offset, seek, TRUE); diff --git a/src/pulse/internal.h b/src/pulse/internal.h index b371bfc2..9545d500 100644 --- a/src/pulse/internal.h +++ b/src/pulse/internal.h @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -66,7 +65,7 @@ struct pa_context { pa_pstream *pstream; pa_pdispatch *pdispatch; - pa_dynarray *record_streams, *playback_streams; + pa_hashmap *record_streams, *playback_streams; PA_LLIST_HEAD(pa_stream, streams); PA_LLIST_HEAD(pa_operation, operations); diff --git a/src/pulse/stream.c b/src/pulse/stream.c index 4dea5670..40812566 100644 --- a/src/pulse/stream.c +++ b/src/pulse/stream.c @@ -199,7 +199,7 @@ static void stream_unlink(pa_stream *s) { pa_pdispatch_unregister_reply(s->context->pdispatch, s); if (s->channel_valid) { - pa_dynarray_put((s->direction == PA_STREAM_PLAYBACK) ? s->context->playback_streams : s->context->record_streams, s->channel, NULL); + pa_hashmap_remove((s->direction == PA_STREAM_PLAYBACK) ? s->context->playback_streams : s->context->record_streams, PA_UINT32_TO_PTR(s->channel)); s->channel = 0; s->channel_valid = FALSE; } @@ -354,7 +354,7 @@ void pa_command_stream_killed(pa_pdispatch *pd, uint32_t command, uint32_t tag, goto finish; } - if (!(s = pa_dynarray_get(command == PA_COMMAND_PLAYBACK_STREAM_KILLED ? c->playback_streams : c->record_streams, channel))) + if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_STREAM_KILLED ? c->playback_streams : c->record_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -474,7 +474,7 @@ void pa_command_stream_moved(pa_pdispatch *pd, uint32_t command, uint32_t tag, p goto finish; } - if (!(s = pa_dynarray_get(command == PA_COMMAND_PLAYBACK_STREAM_MOVED ? c->playback_streams : c->record_streams, channel))) + if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_STREAM_MOVED ? c->playback_streams : c->record_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -557,7 +557,7 @@ void pa_command_stream_buffer_attr(pa_pdispatch *pd, uint32_t command, uint32_t goto finish; } - if (!(s = pa_dynarray_get(command == PA_COMMAND_PLAYBACK_BUFFER_ATTR_CHANGED ? c->playback_streams : c->record_streams, channel))) + if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_BUFFER_ATTR_CHANGED ? c->playback_streams : c->record_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -609,7 +609,7 @@ void pa_command_stream_suspended(pa_pdispatch *pd, uint32_t command, uint32_t ta goto finish; } - if (!(s = pa_dynarray_get(command == PA_COMMAND_PLAYBACK_STREAM_SUSPENDED ? c->playback_streams : c->record_streams, channel))) + if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_STREAM_SUSPENDED ? c->playback_streams : c->record_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -651,7 +651,7 @@ void pa_command_stream_started(pa_pdispatch *pd, uint32_t command, uint32_t tag, goto finish; } - if (!(s = pa_dynarray_get(c->playback_streams, channel))) + if (!(s = pa_hashmap_get(c->playback_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -697,7 +697,7 @@ void pa_command_stream_event(pa_pdispatch *pd, uint32_t command, uint32_t tag, p goto finish; } - if (!(s = pa_dynarray_get(command == PA_COMMAND_PLAYBACK_STREAM_EVENT ? c->playback_streams : c->record_streams, channel))) + if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_STREAM_EVENT ? c->playback_streams : c->record_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -733,7 +733,7 @@ void pa_command_request(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tag goto finish; } - if (!(s = pa_dynarray_get(c->playback_streams, channel))) + if (!(s = pa_hashmap_get(c->playback_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -767,7 +767,7 @@ void pa_command_overflow_or_underflow(pa_pdispatch *pd, uint32_t command, uint32 goto finish; } - if (!(s = pa_dynarray_get(c->playback_streams, channel))) + if (!(s = pa_hashmap_get(c->playback_streams, PA_UINT32_TO_PTR(channel)))) goto finish; if (s->state != PA_STREAM_READY) @@ -997,7 +997,7 @@ void pa_create_stream_callback(pa_pdispatch *pd, uint32_t command, uint32_t tag, } s->channel_valid = TRUE; - pa_dynarray_put((s->direction == PA_STREAM_RECORD) ? s->context->record_streams : s->context->playback_streams, s->channel, s); + pa_hashmap_put((s->direction == PA_STREAM_RECORD) ? s->context->record_streams : s->context->playback_streams, PA_UINT32_TO_PTR(s->channel), s); create_stream_complete(s); -- cgit From 7b1b68ce2cfbc735065f1af45811f33a8a6bf6ea Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 15:28:36 +0200 Subject: dbus: Handle the cases when a non-existing interface is detected in an incoming message. --- src/pulsecore/protocol-dbus.c | 8 ++++++++ src/pulsecore/protocol-dbus.h | 1 + 2 files changed, 9 insertions(+) diff --git a/src/pulsecore/protocol-dbus.c b/src/pulsecore/protocol-dbus.c index e427bb19..95518a13 100644 --- a/src/pulsecore/protocol-dbus.c +++ b/src/pulsecore/protocol-dbus.c @@ -552,10 +552,18 @@ static DBusHandlerResult handle_message_cb(DBusConnection *connection, DBusMessa pa_dbus_send_error(connection, message, DBUS_ERROR_UNKNOWN_METHOD, "No such method: %s", call_info.method); break; + case NO_SUCH_INTERFACE: + pa_dbus_send_error(connection, message, PA_DBUS_ERROR_NO_SUCH_INTERFACE, "No such interface: %s", call_info.interface); + break; + case NO_SUCH_PROPERTY: pa_dbus_send_error(connection, message, PA_DBUS_ERROR_NO_SUCH_PROPERTY, "No such property: %s", call_info.property); break; + case NO_SUCH_PROPERTY_INTERFACE: + pa_dbus_send_error(connection, message, PA_DBUS_ERROR_NO_SUCH_INTERFACE, "No such property interface: %s", call_info.property_interface); + break; + case INVALID_METHOD_SIG: pa_dbus_send_error(connection, message, DBUS_ERROR_INVALID_ARGS, "Invalid signature for method %s: '%s'. Expected '%s'.", diff --git a/src/pulsecore/protocol-dbus.h b/src/pulsecore/protocol-dbus.h index 6d100f7c..89999337 100644 --- a/src/pulsecore/protocol-dbus.h +++ b/src/pulsecore/protocol-dbus.h @@ -35,6 +35,7 @@ #define PA_DBUS_CORE_INTERFACE "org.PulseAudio.Core1" #define PA_DBUS_CORE_OBJECT_PATH "/org/pulseaudio/core1" +#define PA_DBUS_ERROR_NO_SUCH_INTERFACE PA_DBUS_CORE_INTERFACE ".NoSuchInterfaceError" #define PA_DBUS_ERROR_NO_SUCH_PROPERTY PA_DBUS_CORE_INTERFACE ".NoSuchPropertyError" #define PA_DBUS_ERROR_NOT_FOUND PA_DBUS_CORE_INTERFACE ".NotFoundError" -- cgit From e785f728a54b5dc4ad25373b262d35babe8221f6 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 15:30:01 +0200 Subject: dbus: Add a missing break statement in handle_message_cb(). --- src/pulsecore/protocol-dbus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pulsecore/protocol-dbus.c b/src/pulsecore/protocol-dbus.c index 95518a13..582bc658 100644 --- a/src/pulsecore/protocol-dbus.c +++ b/src/pulsecore/protocol-dbus.c @@ -574,6 +574,7 @@ static DBusHandlerResult handle_message_cb(DBusConnection *connection, DBusMessa pa_dbus_send_error(connection, message, DBUS_ERROR_INVALID_ARGS, "Invalid signature for property %s: '%s'. Expected '%s'.", call_info.property, call_info.property_sig, call_info.expected_property_sig); + break; default: pa_assert_not_reached(); -- cgit From a6b7ac68268e55ec8b4ee7fa9e930b86e17fae85 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 15:31:11 +0200 Subject: stream-restore: Fix a few assertion misuses with the D-Bus code. --- src/modules/module-stream-restore.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index 02c312e3..ce923621 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -647,7 +647,7 @@ static void handle_add_entry(DBusConnection *conn, DBusMessage *msg, void *userd } else { dbus_entry = dbus_entry_new(u, name); - pa_assert(pa_hashmap_put(u->dbus_entries, dbus_entry->entry_name, dbus_entry) >= 0); + pa_assert_se(pa_hashmap_put(u->dbus_entries, dbus_entry->entry_name, dbus_entry) == 0); e->muted_valid = TRUE; e->volume_valid = !!map.channels; @@ -1245,10 +1245,10 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3 #ifdef HAVE_DBUS if (created_new_entry) { de = dbus_entry_new(u, name); - pa_hashmap_put(u->dbus_entries, de->entry_name, de); + pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0); send_new_entry_signal(de); } else { - pa_assert((de = pa_hashmap_get(u->dbus_entries, name))); + pa_assert_se(de = pa_hashmap_get(u->dbus_entries, name)); if (device_updated) send_device_updated_signal(de, &entry); @@ -1859,7 +1859,7 @@ static int extension_cb(pa_native_protocol *p, pa_module *m, pa_native_connectio } else { de = dbus_entry_new(u, name); - pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de)); + pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0); send_new_entry_signal(de); } #endif @@ -2050,7 +2050,7 @@ int pa__init(pa_module*m) { pa_datum_free(&key); de = dbus_entry_new(u, name); - pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) >= 0); + pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0); pa_xfree(name); -- cgit From 00debf42437402dadbe5f0a5ff284582c4399aec Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 15:32:23 +0200 Subject: stream-restore: Add a missing pa_xnew0() call in handle_add_entry(). --- src/modules/module-stream-restore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index ce923621..a1273fe9 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -649,6 +649,7 @@ static void handle_add_entry(DBusConnection *conn, DBusMessage *msg, void *userd dbus_entry = dbus_entry_new(u, name); pa_assert_se(pa_hashmap_put(u->dbus_entries, dbus_entry->entry_name, dbus_entry) == 0); + e = pa_xnew0(struct entry, 1); e->muted_valid = TRUE; e->volume_valid = !!map.channels; e->device_valid = !!device[0]; -- cgit From f42022a7d3157a3f3346b49c749d2262abea34c4 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Thu, 3 Dec 2009 15:34:26 +0200 Subject: stream-restore: At startup, create dbus entries only for valid database entries. --- src/modules/module-stream-restore.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index a1273fe9..becdb54a 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -2044,14 +2044,19 @@ int pa__init(pa_module*m) { pa_datum next_key; char *name; struct dbus_entry *de; + struct entry *e; done = !pa_database_next(u->database, &key, &next_key, NULL); name = pa_xstrndup(key.data, key.size); pa_datum_free(&key); - de = dbus_entry_new(u, name); - pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0); + /* Use read_entry() for checking that the entry is valid. */ + if ((e = read_entry(u, name))) { + de = dbus_entry_new(u, name); + pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0); + pa_xfree(e); + } pa_xfree(name); -- cgit From 978d33b609969c3b9bbbd759e0f11aaf856c80cf Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Sun, 6 Dec 2009 12:20:53 +0530 Subject: Mark shared variables as volatile 'n_waiting' and 'n_waiting_for_accept' may be accessed from mulitple threads, and thus need to be marked as volatile to suppres certain compiler optimisations. All uses are protected by a mutex, so we don't need to worry about cache issues (added documentation for this as well). This addresses bug #738. --- src/pulse/thread-mainloop.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pulse/thread-mainloop.c b/src/pulse/thread-mainloop.c index a2b98ce1..14ed9264 100644 --- a/src/pulse/thread-mainloop.c +++ b/src/pulse/thread-mainloop.c @@ -51,7 +51,7 @@ struct pa_threaded_mainloop { pa_mainloop *real_mainloop; - int n_waiting, n_waiting_for_accept; + volatile int n_waiting, n_waiting_for_accept; pa_thread* thread; pa_mutex* mutex; @@ -185,6 +185,7 @@ void pa_threaded_mainloop_unlock(pa_threaded_mainloop *m) { pa_mutex_unlock(m->mutex); } +/* Called with the lock taken */ void pa_threaded_mainloop_signal(pa_threaded_mainloop *m, int wait_for_accept) { pa_assert(m); @@ -198,6 +199,7 @@ void pa_threaded_mainloop_signal(pa_threaded_mainloop *m, int wait_for_accept) { } } +/* Called with the lock taken */ void pa_threaded_mainloop_wait(pa_threaded_mainloop *m) { pa_assert(m); @@ -212,6 +214,7 @@ void pa_threaded_mainloop_wait(pa_threaded_mainloop *m) { m->n_waiting --; } +/* Called with the lock taken */ void pa_threaded_mainloop_accept(pa_threaded_mainloop *m) { pa_assert(m); -- cgit From e8a5746f2fcae59bfd18d39b621509b3ef130453 Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Sun, 6 Dec 2009 12:31:25 +0530 Subject: Add a configure option to change 'udevrulesdir' This patch serves two purposes: 1) Allows something other than the de-facto standard udev rules dir or /lib/udev/rules.d to be used (the udev build system allows you to customise this) 2) Allows a prefixed, non-root install (right now, the /lib/... path is hard-coded into the build system --- configure.ac | 7 +++++++ src/Makefile.am | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index dfbd9bc0..af60fff8 100644 --- a/configure.ac +++ b/configure.ac @@ -1395,6 +1395,13 @@ AC_ARG_WITH( AC_SUBST(modlibexecdir) +AC_ARG_WITH( + [udev-rules-dir], + AS_HELP_STRING([--with-udev-rules-dir],[Directory where to install udev rules to (defaults to /lib/udev/rules.d)]), + [udevrulesdir=$withval], [udevrulesdir="/lib/udev/rules.d"]) + +AC_SUBST(udevrulesdir) + AC_ARG_ENABLE( [force-preopen], AS_HELP_STRING([--enable-force-preopen],[Preopen modules, even when dlopen() is supported.]), diff --git a/src/Makefile.am b/src/Makefile.am index 3be28692..11826a42 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -31,7 +31,6 @@ pulselibexecdir=$(libexecdir)/pulse xdgautostartdir=$(sysconfdir)/xdg/autostart alsaprofilesetsdir=$(datadir)/pulseaudio/alsa-mixer/profile-sets alsapathsdir=$(datadir)/pulseaudio/alsa-mixer/paths -udevrulesdir=/lib/udev/rules.d dbuspolicydir=$(sysconfdir)/dbus-1/system.d ################################### -- cgit