Bug 72213

Created attachment 90073 [details] [review]
Fix the "Fd 5 did not have the close-on-exec flag set!" test warning

(Cc'ing semi-recent contributors from FreeBSD, NetBSD.)

How long has each of the *BSD family had O_CLOEXEC? In other words, do we still need to support any systems that have kqueue but not O_CLOEXEC?

If in doubt, I assume we probably want the usual portability goo:

-          fd = open (new_dirs[i], O_RDONLY);
+#ifdef O_CLOEXEC
+          fd = open (new_dirs[i], O_RDONLY | O_CLOEXEC);
+
+          if (retval < 0 && errno == EINVAL)
+#endif
+            fd = open (new_dirs[i], O_RDONLY);
+
+          _dbus_fd_set_close_on_exec (fd);

Elsewhere in libdbus, we sometimes avoid _dbus_fd_set_close_on_exec() if it would be a no-op, to save a syscall - but this is something that happens O(1) times per bus daemon, so life's too short to worry about syscall efficiency.

Indeed, we could just add the _dbus_fd_set_close_on_exec() call instead of using O_CLOEXEC at all. This code only executes in the dbus-daemon, which is single-threaded, so there's no thread-safety benefit to using O_CLOEXEC.

(In reply to comment #2)
> (Cc'ing semi-recent contributors from FreeBSD, NetBSD.)

Sorry, make that FreeBSD and OpenBSD. If anyone else has any interest in keeping dbus working on *BSD (presumably it has some sort of maintainer in each ports tree?), please direct them to this bug tracker.

(In reply to comment #1)
> Fix the "Fd 5 did not have the close-on-exec flag set!" test warning

Does this also fix the test failure, or do the tests still fail?

(In reply to comment #2)
> (Cc'ing semi-recent contributors from FreeBSD, NetBSD.)
> 
> How long has each of the *BSD family had O_CLOEXEC? In other words, do we
> still need to support any systems that have kqueue but not O_CLOEXEC?

In OpenBSD we have it since more than 2 years -- so as far as OpenBSD is concerned, there is no need to support kqueue but not O_CLOEXEC.

(In reply to comment #3)
> (In reply to comment #2)
> > (Cc'ing semi-recent contributors from FreeBSD, NetBSD.)
> 
> Sorry, make that FreeBSD and OpenBSD. If anyone else has any interest in
> keeping dbus working on *BSD (presumably it has some sort of maintainer in
> each ports tree?), please direct them to this bug tracker.
> 
> (In reply to comment #1)
> > Fix the "Fd 5 did not have the close-on-exec flag set!" test warning
> 
> Does this also fix the test failure, or do the tests still fail?

Yes, still fail with 

"
Failed to activate service 'org.freedesktop.DBus.TestSuiteEchoService': timed out
Did not expect error org.freedesktop.DBus.Error.TimedOut
"

haven't work out a patch so far, seems the activated test-service blocking at wait "Hello" reply from message bus. So the test get a "TimedOut" error.

(In reply to comment #4)
> (In reply to comment #2)
> > (Cc'ing semi-recent contributors from FreeBSD, NetBSD.)
> > 
> > How long has each of the *BSD family had O_CLOEXEC? In other words, do we
> > still need to support any systems that have kqueue but not O_CLOEXEC?
> 
> In OpenBSD we have it since more than 2 years -- so as far as OpenBSD is
> concerned, there is no need to support kqueue but not O_CLOEXEC.

Great to know that from OpenBSD, I'm not sure since when O_CLOEXEC got supported in different *BSD, and its (on FreeBSD 9.1) man page doesn't tell me more, so I just assume it's well supported.

(In reply to comment #2)
> (Cc'ing semi-recent contributors from FreeBSD, NetBSD.)
> 
> How long has each of the *BSD family had O_CLOEXEC? In other words, do we
> still need to support any systems that have kqueue but not O_CLOEXEC?
> 
> If in doubt, I assume we probably want the usual portability goo:
> 
> -          fd = open (new_dirs[i], O_RDONLY);
> +#ifdef O_CLOEXEC
> +          fd = open (new_dirs[i], O_RDONLY | O_CLOEXEC);
> +
> +          if (retval < 0 && errno == EINVAL)
> +#endif
> +            fd = open (new_dirs[i], O_RDONLY);
> +
> +          _dbus_fd_set_close_on_exec (fd);
> 
> Elsewhere in libdbus, we sometimes avoid _dbus_fd_set_close_on_exec() if it
> would be a no-op, to save a syscall - but this is something that happens
> O(1) times per bus daemon, so life's too short to worry about syscall
> efficiency.
> 
> Indeed, we could just add the _dbus_fd_set_close_on_exec() call instead of
> using O_CLOEXEC at all. This code only executes in the dbus-daemon, which is
> single-threaded, so there's no thread-safety benefit to using O_CLOEXEC.

Agree, Seems a reasonable solution if we can not get enough information from *BSD community.

O_CLOEXEC has been supported in FreeBSD since 8.X, which is the earliest supported version at the moment, so you can consider it properly supported there.  It was added to FreeBSD over two years ago

Opened a separate bug to track the timed out failure, see #bug72251.

Hi. Is there any information missing or can this or a variation or it be pushed?

Fixed in git for 1.7.10, thanks.

(In reply to comment #2)
> How long has each of the *BSD family had O_CLOEXEC? In other words, do we
> still need to support any systems that have kqueue but not O_CLOEXEC?

This turns out to break Mac OS X 10.6 (Bug #77032).