GNOME Bugzilla – Bug 648195
outputselector: make it "swap-safe"
Last modified: 2014-11-25 20:32:42 UTC
Created attachment 186266 [details] test-case that defines the desired behavior With this patch, setting active-pad blocks against any activity on that pad. After you have sat active-pad, you are guaranteed that all pad-traffic, aka. chain, bufferalloc and event, have ceased on the pad being switched from. This is very useful for dynamic pipelinbuilding, where you now safely can swap-out, unlink and remove any element without having to worry about a push or a pad-alloc sneaking in at the last minute, hitting elements that are on their way out. All pad-traffic is still kept non-serialized, aka, you can have a pad-alloc while a push in in progress, while an OOB-event is underway. Also some general cleanup and bugfixes for MT-safety. Attached is a test-case for this behavior, as well as a proposed patch.
Created attachment 186267 [details] [review] patch
> With this patch, setting active-pad blocks against any activity on that pad. That sounds like it might cause problems with backwards-compatibility ..
(In reply to comment #2) > > With this patch, setting active-pad blocks against any activity on that pad. > > That sounds like it might cause problems with backwards-compatibility .. I thought about that. We did an external trick to make it "half-safe" (pushes was OK, OOB-events and pad-allocs was not), and that trick could not be used anymore. Aka, it was not compatible with our own code. However, that ment removing a lot of hacks trying to do the job that outputselector should have done in the first place (IMO). I think most users of outputselector would feel the same way. It is now guaranteed to be safe, wheras before it was practically impossible to swap something out safely. But of course I am happy to modify the patch if there are any actual regressions. I'll even write a test-case for it :)
Created attachment 186271 [details] [review] patch Found some unused variables in the previous patch, cleaned them up
Created attachment 186289 [details] [review] patch with fixed author
Just a comment on the one proper regression this change will have: If you use pad-blocking on the pad that outputselector is swapping from, and then go on to set a new active pad, you will deadlock with this patch, since the swapping will wait for the chain-method to return, something it never will, since it is being blocked. The reason one would use pad-blocking with outputselector is of course that it was not swap-safe in the first place, and it is an external way of hacking it to do the right thing. However, using pad-blocking have some serious drawbacks as well. If there never arrives a buffer on the pad being blocked, you can never safely swap, and you will end up potentially waiting forever. Using this patch then will enable you to remove the pad-blocking hack, and have the comfort of knowing that it is now 100% safe, and not having to worry about the potential pad-blocking pitfalls.
Is there something that can be done to avoid the pad-blocking regression? Doesn't sound like a good idea
Is this still an issue/concern for 1.0 ? The blocking/probing system has changed quite a bit.
Håvard, ping.
Havard, ping
Pong. I think the value from this patch is in the attached test. If someone is feeling adventurous, porting that to 1.0 and see if the current outputselector passes would be a confirmation that this is no longer needed. If not, there *could* still be a need for swap-safe outputselectors. :) But feel free to close.
Well, let's close this for the time being. I'm sure you or someone else will file a new bug for 1.x if you notice any issues after porting your stuff to 1.x.