[glib/th/g-close-ebadf] gstdio: fail assertion in g_close() for invalid file descriptor (EBADF)
- From: Thomas Haller <thaller src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib/th/g-close-ebadf] gstdio: fail assertion in g_close() for invalid file descriptor (EBADF)
- Date: Mon, 17 Oct 2022 14:31:35 +0000 (UTC)
commit 2a996ab2fddbf43d346b1b35aa98840cc25fa502
Author: Thomas Haller <thaller redhat com>
Date: Mon Oct 17 16:23:35 2022 +0200
gstdio: fail assertion in g_close() for invalid file descriptor (EBADF)
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.
glib/gstdio.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
---
diff --git a/glib/gstdio.c b/glib/gstdio.c
index f2d58134ef..dd246e5267 100644
--- a/glib/gstdio.c
+++ b/glib/gstdio.c
@@ -1766,26 +1766,40 @@ 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...
+ *
+ * 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));
errno = errsv;
+
+ /* Closing an 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_return_val_if_fail(fd < 0 || errsv != EBADF, FALSE);
+
return FALSE;
}
+
return TRUE;
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]