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 757596 - filesink: left in half cleaned in case of fclose failure
filesink: left in half cleaned in case of fclose failure
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.x
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-04 16:34 UTC by Anton Bondarenko (antonbo)
Modified: 2015-11-05 10:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed fix #1 (872 bytes, patch)
2015-11-04 16:34 UTC, Anton Bondarenko (antonbo)
none Details | Review
Same as "Proposed fix #1", but in git-format style with commit message (2.30 KB, patch)
2015-11-05 09:01 UTC, Anton Bondarenko (antonbo)
committed Details | Review

Description Anton Bondarenko (antonbo) 2015-11-04 16:34:00 UTC
Created attachment 314838 [details] [review]
Proposed fix #1

Sometimes filesink cleanup during stop may fail due to fclose error. In this case element left in partial cleanup state with no file opened but still holding old file descriptor and buffer. It's not possible to modify location of a file due to that and next start may cause old file overwrite.

According to man page and POSIX standard about fclose behavior (extract):
--------------------------
The fclose() function shall cause the stream pointed to by stream to be flushed and the associated file to be closed.
...
Whether or not the call succeeds, the stream shall be disassociated from the file and any buffer set by the setbuf() or setvbuf() function shall be disassociated from the stream. 
...
The fclose() function shall perform the equivalent of a close() on the file descriptor that is associated with the stream pointed to by stream.

After the call to fclose(), any use of stream results in undefined behavior.
--------------------------

So I think we can assume file is closed even if fclose give an error.
Please find attached patch with proposed fix.
Comment 1 Sebastian Dröge (slomo) 2015-11-04 21:22:07 UTC
Comment on attachment 314838 [details] [review]
Proposed fix #1

Makes sense, but can you attach this as a "git format-patch" style patch with a proper commit message that explains what you're doing there and why?
Comment 2 Anton Bondarenko (antonbo) 2015-11-05 09:01:20 UTC
Created attachment 314872 [details] [review]
Same as "Proposed fix #1", but in git-format style with commit message

Replaced generic patch with 'git format' patch
Comment 3 Sebastian Dröge (slomo) 2015-11-05 09:19:15 UTC
commit f468eb7db023e110c06ad163cf48908107cf0382
Author: Anton Bondarenko <antonbo@axis.com>
Date:   Thu Nov 5 08:56:43 2015 +0100

    filesink: continue element cleanup even if fclose fails
    
    Sometimes filesink cleanup during stop may fail due to fclose error.
    In this case object left partial cleanup with no file opened
    but still holding old file descriptor.
    
    It's not possible to change location property in a such state,
    so next start will cause old file overwrite if 'append' does not set.
    
    According to man page and POSIX standard about fclose behavior(extract):
    ------------------------------------------------------------------------
    The fclose() function shall cause the stream pointed to by stream
    to be flushed and the associated file to be closed.
    ...
    Whether or not the call succeeds, the stream shall be disassociated
    from the file and any buffer set by the setbuf() or setvbuf()
    function shall be disassociated from the stream.
    ...
    The fclose() function shall perform the equivalent of a close()
    on the file descriptor that is associated with the stream
    pointed to by stream.
    
    After the call to fclose(), any use of stream results
    in undefined behavior.
    ------------------------------------------------------------------------
    
    So file is in 'closed' state no matter if fclose succeed or not.
    And cleanup could be continued.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757596
Comment 4 Anton Bondarenko (antonbo) 2015-11-05 10:31:42 UTC
Thanks for a fast response.