Bug in test-fcntl.c

Bug in test-fcntl.c

[Nicholas Gaya]

Hello,

I believe I have found a bug in gnulib's test-fcntl.c. In order to test the functionality of fcntl with FD_DUPFD_CLOEXEC, the test dups an open file descriptor using the flag and execs a child process to check that the file descriptor gets closed on exec. The problem is that the current test code assumes that the duplicated file descriptor will have constant value 10, which will not be true if fd 10 is already in use (which happens to be the case in the testing environment where I ran into this bug).

To fix this, we can pass the file descriptor returned by fcntl as an argument to the child process. Please see this proposed patch for details: https://gist.github.com/nickgaya/2a70ebdce8c020481d3e5b6c3696fe7b

Best,
Nicholas Gaya

Re: Bug in test-fcntl.c

[Bruno Haible]

Hi,

Nicholas Gaya wrote:
> fd 10 is already in use (which happens to be the case in the testing
> environment where I ran into this bug)

3 questions:
1) What is this environment that leaves fd 10 open when it starts a process?
2) For which purpose does it use this fd?
3) What would happen if we close fd 10 in test-fcntl.c?

Bruno

Re: Bug in test-fcntl.c

[Nicholas Gaya]

I ran into this issue when running tests for the MacPorts port of oath-toolkit (https://ports.macports.org/port/oath-toolkit/) on Mac OS 10.14.6. I don't know the specific reason, but it seems like the test process inherits a lot of open files from its parent, as the actual fd returned by fcntl is 67.

Closing fd 10 is a good suggestion, as it's simpler than having to convert the fd to/from a string to pass it to the child process. I updated the patch file with this approach.

> On May 8, 2021, at 7:03 PM, Bruno Haible <bruno@clisp.org> wrote:
> 
> Hi,
> 
> Nicholas Gaya wrote:
>> fd 10 is already in use (which happens to be the case in the testing
>> environment where I ran into this bug)
> 
> 3 questions:
> 1) What is this environment that leaves fd 10 open when it starts a process?
> 2) For which purpose does it use this fd?
> 3) What would happen if we close fd 10 in test-fcntl.c?
> 
> Bruno
> 

Re: Bug in test-fcntl.c

[Bruno Haible]

Nicholas Gaya wrote:
> it seems like the test process inherits a lot of open files from its parent,
> as the actual fd returned by fcntl is 67.

That seems excessive. What is the chain of parent, grandparent etc. processes?

Bruno

Re: Bug in test-fcntl.c

[Nicholas Gaya]

The process hierarchy starts with tclsh (MacPorts), then a bunch of layers of make and sh (from oath-toolkit's recursive `make check`), and finally the test-fcntl executable. I was using MacPorts' "trace mode", which seems to be responsible for the particularly high number of inherited open files, but without trace mode I still run into the issue; the returned fd is 11 in this case.

But I don't really want to focus on my specific use-case (for which I already have a workaround).  I think the patch is warranted just on the basis that it makes the test a bit more robust, with minimal drawbacks.

> On May 9, 2021, at 5:24 AM, Bruno Haible <bruno@clisp.org> wrote:
> 
> Nicholas Gaya wrote:
>> it seems like the test process inherits a lot of open files from its parent,
>> as the actual fd returned by fcntl is 67.
> 
> That seems excessive. What is the chain of parent, grandparent etc. processes?
> 
> Bruno
> 

Re: Bug in test-fcntl.c

[Bruno Haible]

Nicholas Gaya wrote:
> The process hierarchy starts with tclsh (MacPorts), then a bunch of layers of make and sh (from oath-toolkit's recursive `make check`), and finally the test-fcntl executable. I was using MacPorts' "trace mode", which seems to be responsible for the particularly high number of inherited open files, but without trace mode I still run into the issue; the returned fd is 11 in this case.

Thanks; that gives a hint also who is eating up the file descriptors 3 to 10.

> But I don't really want to focus on my specific use-case (for which I already have a workaround).  I think the patch is warranted just on the basis that it makes the test a bit more robust, with minimal drawbacks.

Yes. But I wanted to have a description of the cause, that I can include in
comments.

Done. Thanks for the report!


2021-05-14  Bruno Haible  <bruno@clisp.org>

	fcntl tests: Avoid failure in MacPorts.
	Reported by Nicholas Gaya <nicholasgaya@gmail.com> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2021-05/msg00014.html>.
	* tests/test-fcntl.c (main): Close fd 10 before assuming that it is
	closed.
	* tests/test-execute-main.c: Update comment.

diff --git a/tests/test-execute-main.c b/tests/test-execute-main.c
index a6a9fe4..372ff1d 100644
--- a/tests/test-execute-main.c
+++ b/tests/test-execute-main.c
@@ -69,7 +69,8 @@ main (int argc, char *argv[])
          Such file descriptors have been seen:
            - with GNU make, when invoked as 'make -j N' with j > 1,
            - in some versions of the KDE desktop environment,
-           - on NetBSD.
+           - on NetBSD,
+           - in MacPorts with the "trace mode" enabled.
        */
       #if HAVE_CLOSE_RANGE
       if (close_range (3, 20 - 1, 0) < 0)
diff --git a/tests/test-fcntl.c b/tests/test-fcntl.c
index caf629d..cb834b4 100644
--- a/tests/test-fcntl.c
+++ b/tests/test-fcntl.c
@@ -415,6 +415,16 @@ main (int argc, char *argv[])
   ASSERT (close (fd) == 0);
   ASSERT (unlink (file) == 0);
 
+  /* Close file descriptors that may have been inherited from the parent
+     process and that would cause failures below.
+     Such file descriptors have been seen:
+       - with GNU make, when invoked as 'make -j N' with j > 1,
+       - in some versions of the KDE desktop environment,
+       - on NetBSD,
+       - in MacPorts with the "trace mode" enabled.
+   */
+  (void) close (10);
+
   /* Test whether F_DUPFD_CLOEXEC is effective.  */
   ASSERT (fcntl (1, F_DUPFD_CLOEXEC, 10) >= 0);
 #if defined _WIN32 && !defined __CYGWIN__