GNOME Bugzilla – Bug 675640
shmsink memory corruption when a client disconnects (more than one client)
Last modified: 2012-10-06 11:43:43 UTC
If more than one shmsrc client is connected to a shmsink a race condition might occur. Supose we have clients C1 and C2 and buffer B (with use_count=2). In pollthread_func client loop (client C1 has disconnected at some point during next steps), - Client C1 decrement the use count of buffer B (sp_writer_recv -> use_count=1). - Client C2 gst_poll_fd_can_read returns FALSE (sp_writer_recv *not* called). - pollthread_func is called again (before gst-shm_sink_render). - Client C1 has closed (sp_writer_close_client -> use_count=0 -> free buffer). <--- ERROR Client C1 has already decremented use count for buffer B. - Client C2 pipe still has buffer B <--- ERROR (it has already been freed)! The attached patch solves this issue by removing the client from the list of buffer users when the use count is decremented for that buffer. This way, when the client closes, it does not try to decrement buffer use count again.
Created attachment 213622 [details] [review] fix for shmsink with multiple clients
To try it: GST_DEBUG=3 gst-launch videotestsrc ! shmsink socket-path=/tmp/shmtest shm-size=125829120 wait-for-connection=FALSE And a couple or more clients with: GST_DEBUG=3 gst-launch shmsrc socket-path=/tmp/shmtest is-live=TRUE do-timestamp=TRUE ! fakesink -v Ctrl-C from one client and the other will disconnect too. The crash does not always occur so you might need to connect and Ctrl-C again more than once. This also leaves the shmsink in a bad state as it segfaults when you Ctrl-C it.
This example works fine for me.. but you're patch looks good. I modified it slightly to run loop before removing the reference and to check that the client was in there and assert() if it wasn't. commit cf8f7a25a0c650b5a6e729ed6e45092a2ba2c8b9 Author: Aleix Conchillo Flaque <aleix@oblong.com> Date: Mon May 7 13:13:34 2012 -0700 shmsink: fix memory corruption when a client disconnects (fixes #675640) Also, add a check to make sure a client isn't dumped twice
Also pushed to 0.11 branch commit 3971ef089cb2e636a720fc9ab3bd3879ee6a24c8 Author: Aleix Conchillo Flaque <aleix@oblong.com> Date: Mon May 7 13:13:34 2012 -0700 shmsink: fix memory corruption when a client disconnects (fixes #675640) Also, add a check to make sure a client isn't dumped twice
(In reply to comment #3) > This example works fine for me.. but you're patch looks good. > > I modified it slightly to run loop before removing the reference and to check > that the client was in there and assert() if it wasn't. > Great, thanks!