Would you have a test-case please? Also what version of PA are you using?

pulseaudio: 0.9.21 (ubuntu lucid)
same happens at home suse 11.3
so it is probably not the pa version

I get this when playing songs in buzztard. I'll dig it, but just filed the bug to make sure it gets fixed before we release next one.

That said, I think the context array should be in initializen when the pulse plugin is initialized and both pulsesrc and sink should use it.

Created attachment 169757 [details] [review]
add asserts to trap the crash

It crashes here, pctx->context is NULL. Dunno yet why:

  /* we need a context and a no stream */
  pctx = gst_pulsering_get_context (pbuf);
  g_assert (!pbuf->stream);
  g_assert (pctx);
  g_assert (pctx->context);

  /* enable event notifications */
  GST_LOG_OBJECT (psink, "subscribing to context events");
  if (!(o = pa_context_subscribe (pctx->context,
              PA_SUBSCRIPTION_MASK_SINK_INPUT, NULL, NULL)))
    goto subscribe_failed;

pulsesink.c:466:gst_pulseringbuffer_open_device:<player> new context with name Buzztard
pulsesink.c:374:gst_pulsering_context_state_cb:<player> got new context state 1
pulsesink.c:500:gst_pulseringbuffer_open_device:<player> context state is now 1
pulsesink.c:374:gst_pulsering_context_state_cb:<player> got new context state 2
pulsesink.c:374:gst_pulsering_context_state_cb:<player> got new context state 3
pulsesink.c:374:gst_pulsering_context_state_cb:<player> got new context state 4
pulsesink.c:385:gst_pulsering_context_state_cb:<player> signaling
pulsesink.c:500:gst_pulseringbuffer_open_device:<player> context state is now 4
pulsesink.c:491:gst_pulseringbuffer_open_device:<player> reusing shared pulseaudio context with name Buzztard
pulsesink.c:500:gst_pulseringbuffer_open_device:<player> context state is now 4
pulsesink.c:294:gst_pulsering_destroy_context:<player> destroying context for pbuf=0x8e06140 'Buzztard'
pulsesink.c:294:gst_pulsering_destroy_context: destroying context for pbuf=0x8e06140 'Buzztard'
pulsesink.c:761:gst_pulseringbuffer_acquire:<player> subscribing to context events

it shows that buzztard triggers the sharing part. what I wonder is why gst_pulsering_destroy_context() is called twice, the 2nd time pbuf seems to be in the process of destruction (has no gst-object name anymore).

open suse also has 0.9.21

This also fails:
gst-launch-0.10 audiotestsrc num-buffers=15 ! pulsesink audiotestsrc num-buffers=25 ! pulsesink

It eitehr hangs or segfaults. It works with released -good. I am now bisecting.

git bisect brought me to:

69a397c32f4baf07a7b2937c610f9e8f383e9ae9 is the first bad commit
commit 69a397c32f4baf07a7b2937c610f9e8f383e9ae9
Author: Philippe Normand <pnormand@igalia.com>
Date:   Mon Aug 16 09:12:04 2010 +0200

    pulsesink: share the PA context between all clients with the same name
    
    Avoid to create a new PA context for each new client by using a hash
    table containing the list of ring-buffers and the shared PA context
    for each client. Doing this will improve application memory usage in
    the cases where multiple pipelines involving multiple pulsesink
    elements are used.
    
    Fixes bug #624338.

That pipeline works fine here, although I use an old PA (0.9.19-0ubuntu4.1) on this laptop running karmic.

(In reply to comment #7)
> That pipeline works fine here, although I use an old PA (0.9.19-0ubuntu4.1) on
> this laptop running karmic.

Nevermind that, got it to hang ;)
Well I guess this is my fault, right? :) Will have a look, sorry for breaking the thing

Created attachment 169895 [details]
valgrind log

in gst_pulseringbuffer_open_device()
it does
    api = pa_threaded_mainloop_get_api (psink->mainloop);
    if (!(pctx->context = pa_context_new (api, pbuf->context_name)))
      goto create_failed;

it seems that the context is cleared when the mainloop that was used to create it is freed in gst_pulsesink_change_state() (see valgrind log).

I wonder if the main_loop that was used to create the context can be keep and only be cleared when the last instance it released.

I have now tried to use extra
pa_context_ref (pctx->context);
pa_context_unref (pctx->context);
for each user of the context, to no avail.

Honestly I doubt the contexy is actualy shareable.

The mainloop itself is not easily shareable either, as it is started/stopped with the state-changes. We would need a patch that shares the mainloop, starts it upon creation and stops it on last unref.

I think we need some changes on the pulseaudio side for this then and should revert the change until there's a solution in PA.

Actually forget the last comment. Sharing the mainloop should be really possible, start it from the first element that goes to READY and stop it from the last element that goes to NULL.

(In reply to comment #13)
> Actually forget the last comment. Sharing the mainloop should be really
> possible, start it from the first element that goes to READY and stop it from
> the last element that goes to NULL.

Thats what I said in IRC. Phil, are you try to implement that change? Otherwise I'll try tomorrow.

(In reply to comment #14)
> (In reply to comment #13)
> > Actually forget the last comment. Sharing the mainloop should be really
> > possible, start it from the first element that goes to READY and stop it from
> > the last element that goes to NULL.
> 
> Thats what I said in IRC. Phil, are you try to implement that change? Otherwise
> I'll try tomorrow.

I had a look yesterday night but today was busy with other stuff, please don't wait on me, tomorrow i'll be offline most of the day too.

Phil, I was not waiting on you for the whole week already. This totally blocks the work on my own app. I already spend all my evening this week on this. But so it is then. I'll try fixing it tomorrow, tonight I am too tired.

commit f62dc6976b611384c98efb37d407b5299daf8c17
Author: Stefan Kost <ensonic@users.sf.net>
Date:   Mon Sep 13 16:24:26 2010 +0300

    pulsesink: rework context sharing
    
    We also need to share the main-loop threads as this owns the context. Thus have
    a class wide main-loop thread. From this we create a context per client-name.
    Instead of always looking up the context, we keep this with the instance. The
    reverse mapping is only needed in pulse singal handlers. This saves a lot of
    locking. Also one signal handler becomes simpler as ther eis only one mainloop
    to notify.
    
    Now valgind happy - no leaks, no bad reads/writes.
    
    This reverts major parts of commit 69a397c32f4baf07a7b2937c610f9e8f383e9ae9.
    
    Fixes #628996