Skip to content

gspawn: Ignore invalid FDs when using safe_fdwalk()

Philip Withnall requested to merge pwithnall/glib:valgrind-fd-fun into main

In safe_closefrom(), we thought it would be OK to assert that an FD being closed is valid, when using safe_fdwalk(), as it only walks over known-valid FDs.

However, there are two situations where that might not be true:

  1. The FD is closed from another thread between being chosen for closing by safe_fdwalk() and g_close() actually being called on it. This is unlikely and I haven’t seen it in practice, but it is possible because using safe_fdwalk() circumvents the normal static refcounting of FDs. See discussion below
  2. The program is being run under valgrind. Valgrind opens some FDs for its own use which are ≥1024, and it emulates a lowered soft limit on FDs. So if we were to use safe_fdwalk_with_invalid_fds() it would see the lowered soft limit and not try to close valgrind’s internal FDs. However, safe_fdwalk() looks at /proc, which valgrind does not emulate, so it sees the secret valgrind internal FDs, and then tries to close them. Valgrind doesn’t like this, prints “Warning: invalid file descriptor 1024 in syscall close()” and returns EBADF. That return value causes g_close() to warn about faulty FD refcounting, and that causes unit test failures.

Fix that by relaxing our assumptions about FD validity: use the close_func_with_invalid_fds() call back for closing FDs from safe_fdwalk(), rather than using close_func(). That will ignore EBADF return values.

This should fix valgrind failures like this one: https://gitlab.gnome.org/GNOME/glib/-/jobs/2389977

Related prior art: https://bugs.freedesktop.org/show_bug.cgi?id=99839

Signed-off-by: Philip Withnall pwithnall@endlessos.org

Edited by Philip Withnall

Merge request reports