[pulseaudio-discuss] [PATCH 02/11] pulsecore: srbchannel: Introduce per-client SHM files

[Tanu Kaskinen]

On Sun, 2015-09-20 at 23:28 +0200, Ahmed S. Darwish wrote:
> The PA daemon currently uses a server-wide SHM file for all clients
> sending and receiving commands using the srbchannel.
> 
> To protect the clients from each other, and to provide the necessary
> groundwork later for policy and sandboxing, create the srbchannel SHM
> files on a per-client basis.
> 
> Signed-off-by: Ahmed S. Darwish <darwish.07 at gmail.com>
> ---
>  src/pulsecore/client.c          |  5 +++++
>  src/pulsecore/client.h          |  7 +++++++
>  src/pulsecore/core.c            | 15 +++++----------
>  src/pulsecore/core.h            |  6 ++----
>  src/pulsecore/protocol-native.c | 16 ++++++++++++----
>  5 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/src/pulsecore/client.c b/src/pulsecore/client.c
> index 003bcf8..5d60ad6 100644
> --- a/src/pulsecore/client.c
> +++ b/src/pulsecore/client.c
> @@ -69,6 +69,8 @@ pa_client *pa_client_new(pa_core *core,
> pa_client_new_data *data) {
>      c->sink_inputs = pa_idxset_new(NULL, NULL);
>      c->source_outputs = pa_idxset_new(NULL, NULL);
>  
> +    c->rw_mempool = NULL;

You could a patch that changes

    c = pa_xnew(pa_client, 1);

to

    c = pa_xnew0(pa_client, 1);

so that the struct fields don't need to be initialized to zero
manually.

...hmmm... Is there actually any good reason to have rw_mempool in
pa_client? Could it be moved to the native protocol? It looks like the
only user outside the native protocol is pa_core_maybe_vacuum().

If I understand vacuuming correctly, it means telling the kernel that
the memory in unused memblocks probably won't be used any time soon,
and that we also don't care about the current data in them, so the
kernel can optimize memory usage accordingly. pa_core_maybe_vacuum()
only vacuums when there's no audio moving in the system. When you
change rw_mempools to be created separately for each client, I think it
would make sense to vacuum the per-client mempool every time the client
stops streaming. That logic could live inside the native protocol, so
the core wouldn't have to know about rw_mempools.

This would also get rid of the inconsistency that rw_mempool is created
in protocol-native.c but freed in client.c.

-- 
Tanu