GNOME Bugzilla – Bug 790468
atomicqueue: not MT-safe and racy
Last modified: 2018-11-03 12:43:04 UTC
_push() and _pop() from different threads can result in skips and/or memory corruption.
Created attachment 363869 [details] [review] gst/atomicqueue: add some actual tests of functionality A test the exhibits the behaviour.
We should test with 1 pop threads and multiple push thread (the most common use through buffer pool). Will help figuring out how bad this is.
See GstBus' and GstBufferPool's usage of atomicqueue, you're supposed to add some kind of serialization of operations and waiting with GstPoll around it. There were also some commits for fixing issues around this in both files. But maybe your problem is something else?
That seems to defeat the purpose of an atomic queue no if there's external synchronisation? Especially if there's the potential for memory corruption (which there is).
Yes, but that's what Wim told me last time I looked into this (because of memory corruption in GstBus). By having the synchronization outside the queue, you could potentially compose it better with other things that needs synchronization too. Like waiting for one out of many things instead of blocking on the queue directly. Also the GstPoll is only atomic integer operations, unless it actually has to wait.
This is without any kind of synchronisation at all though. e.g. replacing GAsyncQueue with GstAtomicQueue should not result in memory corruption.
It does because you need synchronization, otherwise you write things that you're just reading, etc.
But we could also make it thread safe. If you have external synchronization it will just make it less often hit the spins, so "in theory" it shouldn't be slower. The documentation is currently completely miss-leading, because it's documented exactly the way Matthew started using it.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/256.