After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 786051 - glupload: EGLImage cache is not thread safe
glupload: EGLImage cache is not thread safe
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.13.x
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 786175
 
 
Reported: 2017-08-09 14:00 UTC by Nicolas Dufresne (ndufresne)
Modified: 2018-11-03 11:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Nicolas Dufresne (ndufresne) 2017-08-09 14:00:09 UTC 
When we receive a DMABuf, we create a cache associated EGLImage. While the wrapper is refcounted, the way we store that EGLImage is not thread safe. The problem is that we don't get and ref the EGLImage atomically when retrieving the qdata. So in between, sometimes the EGLImage get destroyed, and then we ended with use after free crashing GStreamer or Mesa.

The pipeline I use to reproduce currently:

gst-launch-1.0 v4l2src device=/dev/video1 io-mode=dmabuf ! tee name=t \
  t. ! queue ! glsinkbin sink=gtkglsink \
  t. ! queue ! glsinkbin sink=gtkglsink \
  t. ! queue ! glsinkbin sink=gtkglsink \
  t. ! queue ! glsinkbin sink=gtkglsink \
  t. ! queue ! glsinkbin sink=gtkglsink
Comment 1 Julien Isorce 2017-08-10 23:41:23 UTC 
I thought a moment we could use gst_memory_lock (mem, GST_LOCK_FLAG_EXCLUSIVE) ? but in fact it is a try-lock.

Can we guard that for loop (around get_cache/set_cache) with gst_gl_display_lock/unlock ? This should be the same GstGLDisplay instance since it is per pipeline if I am correct.

Or using gst_memory_share somehow ? So that all memories going out of the tee are not the same GstMemory instances as the input. It solves the race while not using a global lock. It will create much more eglimage but these are just references of the same underlying memory.
Comment 2 Nicolas Dufresne (ndufresne) 2017-08-11 00:37:13 UTC 
We need a local cache anyway to implement drain properly (see kmssink recent changes). As soon as you have that, it's easy to stop using qdata. I prefer using a cache per uploader, since it removes the contention.
Comment 3 GStreamer system administrator 2018-11-03 11:59:03 UTC 
-- 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/gst-plugins-base/issues/375.