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 724077 - shm: use shutdown() instead of close()
shm: use shutdown() instead of close()
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-02-10 22:33 UTC by Aleix Conchillo Flaqué
Modified: 2014-02-10 23:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
replace close() by shutdown() (1.33 KB, patch)
2014-02-10 22:35 UTC, Aleix Conchillo Flaqué
committed Details | Review
and also call close() (1.30 KB, patch)
2014-02-10 23:40 UTC, Aleix Conchillo Flaqué
committed Details | Review

Description Aleix Conchillo Flaqué 2014-02-10 22:33:15 UTC
I've hit a problem with the shared memory elements. 

I have 1 shmsink and 2 shmsrc clients. And sometimes it seems close() is not enough on the client side. Replacing close() by shutdown() solves the problem.

Unfortunately, I just know (and have only tried actually) how to reproduce it with one of our apps.

The way I reproduce it is byt creating the clients one after the other (few seconds difference) and closing them at the same time. If I create both clients at the same time and close them at the same time, everything works. I know this explanation sucks and doesn't make much sense, but that's all I know so far.

I don't know why this didn't happen in 0.10, unless something in gstpoll changed. As far as I can see, shm elements look more or less the same, specially the closing socket part.
Comment 1 Aleix Conchillo Flaqué 2014-02-10 22:35:33 UTC
Created attachment 268741 [details] [review]
replace close() by shutdown()
Comment 2 Aleix Conchillo Flaqué 2014-02-10 22:50:11 UTC
In any case, shutdown() seems what we want here, as we want to make sure the other end detects the hang up properly.
Comment 3 Olivier Crête 2014-02-10 23:23:10 UTC
I guess it's because close() really just does a unref on the socket, and the poll() also holds a ref, so maybe that causes the closing to be delayed more than expected. I see no harm in your patch, so I committed it.

commit f5a1ccd0deb8d82c6befab358e942739ed609b25
Author: Aleix Conchillo Flaqué <aleix@oblong.com>
Date:   Mon Feb 10 14:33:49 2014 -0800

    shm: use shutdown() instead of close()
    
    we make sure both ends get notified when the socket is closed by using
    shutdown() instead of close().
    
    https://bugzilla.gnome.org/show_bug.cgi?id=724077
Comment 4 Aleix Conchillo Flaqué 2014-02-10 23:40:58 UTC
Created attachment 268742 [details] [review]
and also call close()

shutdown() doesn't clsoe the file descriptor. se we leak sockets if we don't call close().

Thanks to Dan Kegel for pointing this out.
Comment 5 Aleix Conchillo Flaqué 2014-02-10 23:42:41 UTC
(In reply to comment #4)
> 
> shutdown() doesn't clsoe the file descriptor. se we leak sockets if we don't
> call close().
> 

I also verified this.
Comment 6 Olivier Crête 2014-02-10 23:54:47 UTC
Comment on attachment 268742 [details] [review]
and also call close()

Arg, I shouldn't have missed this, committed too..

commit fd27bdf5f056967193f1af2a97aa3de806606fae
Author: Aleix Conchillo Flaqué <aleix@oblong.com>
Date:   Mon Feb 10 15:38:08 2014 -0800

    shm: call close() after shutdown()
    
    shutdown() doesn't close the file descriptor so we leak sockets if we
    don't call close().
    
    https://bugzilla.gnome.org/show_bug.cgi?id=724077