testhelper: Fix ETXTBSY race when writing executables

On Unix-systems, trying to execve(3P) into an executable which is
currently open in any process as writeable will result in ETXTBSY
errors. We're frequently observing this error in our test suite when
using the `WriteExecutable()` testhelper function.

The root cause of this is a race between fork(3P), execve(3P) and us
still having the file descriptor open for writing. Suppose the following
race where one thread is trying to write an executable and execute it,
and a second thread forking to execute any other executable:

    Thread 1                    Thread 2

    fd = open(O_WRONLY)
                                fork()
    close(fd)
    fork()
    execve(fd)
                                execve()

We call fork(3P) in the second thread after having opened the file as
writeable, which means that the newly created process inherits the
writeable file descriptor. And even though we close it in the first
thread, execve(3P) will fail with ETXTBSY because it's still open in the
second process, which didn't yet call execve(3P) itself and thus hasn't
closed the file descriptor until now.

Fix this race by using file locking such that we know that all writeable
file descriptors must have been closed at the time of returning from
`WriteExecutable()`. Please refer to [1] for further information about
this issue.

[1]: https://github.com/golang/go/issues/22315

1 files changed, 37 insertions, 5 deletions

diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 62e507a4d..a9ebfe62a 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -16,6 +16,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"syscall"
 	"testing"
 	"time"
 
@@ -240,17 +241,48 @@ type Cleanup func()
 func WriteExecutable(t testing.TB, path string, content []byte) string {
 	dir := filepath.Dir(path)
 	require.NoError(t, os.MkdirAll(dir, 0o755))
+	t.Cleanup(func() {
+		assert.NoError(t, os.RemoveAll(dir))
+	})
 
+	// Open the file descriptor and write the script into it. It may happen that any other
+	// Goroutine forks while we hold this writeable file descriptor, and as a consequence we
+	// leak it into the other process. Subsequently, even if we close the file descriptor
+	// ourselves this other process may still hold on to the writeable file descriptor. The
+	// result is that calls to execve(3P) on our just-written file will fail with ETXTBSY,
+	// which is raised when trying to execute a file which is still open to be written to.
+	//
+	// We thus need to perform file locking to ensure that all writeable references to this
+	// file have been closed before returning.
 	executable, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o755)
 	require.NoError(t, err)
-	defer MustClose(t, executable)
-
 	_, err = io.Copy(executable, bytes.NewReader(content))
 	require.NoError(t, err)
 
-	t.Cleanup(func() {
-		assert.NoError(t, os.RemoveAll(dir))
-	})
+	// We now lock the file descriptor for exclusive access. If there was a forked process
+	// holding the writeable file descriptor at this point in time, then it would refer to the
+	// same file descriptor and thus be locked for exclusive access, as well. If we fork after
+	// creating the lock and before closing the writeable file descriptor, then the dup'd file
+	// descriptor would automatically inherit the lock.
+	//
+	// No matter what, after this step any file descriptors referring to this writeable file
+	// descriptor will be exclusively locked.
+	require.NoError(t, syscall.Flock(int(executable.Fd()), syscall.LOCK_EX))
+
+	// We now close this file. The file will be automatically unlocked as soon as all
+	// references to this file descriptor are closed.
+	MustClose(t, executable)
+
+	// We now open the file again, but this time only for reading.
+	executable, err = os.Open(path)
+	require.NoError(t, err)
+
+	// And this time, we try to acquire a shared lock on this file. This call will block until
+	// the exclusive file lock on the above writeable file descriptor has been dropped. So as
+	// soon as we're able to acquire the lock we know that there cannot be any open writeable
+	// file descriptors for this file anymore, and thus we won't get ETXTBSY anymore.
+	require.NoError(t, syscall.Flock(int(executable.Fd()), syscall.LOCK_SH))
+	MustClose(t, executable)
 
 	return path
 }