luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-02-03 17:56:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

gstdio: fail assertion in g_close() for invalid file descriptor (EBADF)

Browse Source 
An application must keep track of the file descriptors that it
has. Closing an invalid, non-negative file descriptor is usually
a bug, because it indicates somebody messed up the tracking.

On a single threaded application it may be fine, but EBADF is always a bug
in a multi threaded application because another thread might race
reusing the bad file descriptor. With GDBus and other glib API, it is very
common that your application has multiple threads running and this is
in fact a bug.

The assertion failure does not necessarily indicate that the bug
is in the caller. It could have been another part of the application
that wrongly closed the file descriptor.
This commit is contained in:
Thomas Haller 2022-10-17 16:23:35 +02:00 committed by Philip Withnall
parent 52917f57ca
commit d5dc7d266f
1 changed files with 42 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
glib/gstdio.c
View File

@ -1757,6 +1757,8 @@ g_utime (const gchar *filename,
* attempt to correctly handle %EINTR, which has platform-specific
* semantics.
*
* It is a bug to call this function with an invalid file descriptor.
*
* Returns: %TRUE on success, %FALSE if there was an error.
*
* Since: 2.36
@ -1766,26 +1768,54 @@ g_close (gint fd,
GError **error)
{
int res;
res = close (fd);
/* Just ignore EINTR for now; a retry loop is the wrong thing to do
* on Linux at least. Anyone who wants to add a conditional check
* for e.g. HP-UX is welcome to do so later...
*
* http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
* https://bugzilla.gnome.org/show_bug.cgi?id=682819
* http://utcc.utoronto.ca/~cks/space/blog/unix/CloseEINTR
* https://sites.google.com/site/michaelsafyan/software-engineering/checkforeintrwheninvokingclosethinkagain
*/
if (G_UNLIKELY (res == -1 && errno == EINTR))
return TRUE;
else if (res == -1)
if (res == -1)
{
int errsv = errno;
if (errsv == EINTR)
{
/* Just ignore EINTR for now; a retry loop is the wrong thing to do
* on Linux at least. Anyone who wants to add a conditional check
* for e.g. HP-UX is welcome to do so later...
*
* https://lwn.net/Articles/576478/
* http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
* https://bugzilla.gnome.org/show_bug.cgi?id=682819
* http://utcc.utoronto.ca/~cks/space/blog/unix/CloseEINTR
* https://sites.google.com/site/michaelsafyan/software-engineering/checkforeintrwheninvokingclosethinkagain
*/
return TRUE;
}
g_set_error_literal (error, G_FILE_ERROR,
g_file_error_from_errno (errsv),
g_strerror (errsv));
if (errsv == EBADF)
{
if (fd >= 0)
{
/* Closing an non-negative, invalid file descriptor is a bug. The bug is
* not necessarily in the caller of g_close(), but somebody else
* might have wrongly closed fd. In any case, there is a serious bug
* somewhere. */
g_critical ("g_close(fd:%d) failed with EBADF. The tracking of file descriptors got messed up", fd);
}
else
{
/* Closing a negative "file descriptor" is less problematic. It's still a nonsensical action
* from the caller. Assert against that too. */
g_critical ("g_close(fd:%d) failed with EBADF. This is not a valid file descriptor", fd);
}
}
errno = errsv;
return FALSE;
}
return TRUE;
}