test: Grab FORK_MTX when unmounting or forking in the mount tests #2386
Conversation
unmounting can fail if a child process inherited a file a file descriptor we opened in temporary mount point Should fix nix-rust#2369
|
Hi, thanks for working on this! What about we set the close-on-exec flag for the test files in that bind-mount fs (as well as the fds in other mount tests), I kinda think this would be more straightforward, thoughts? |
|
The close-on exec-flag will not prevent the issue, because the issue is not exec, but fork: it copies any opened file descriptors and then they live until exit or exec. if a file descriptor to something on the mountpoint is forked, unmount will fail. |
You are right, there is still a timespan between |
test/mount/test_mount.rs
Outdated
| @@ -89,6 +91,8 @@ fn test_mount_rdonly_disallows_write() { | |||
| .unwrap() | |||
| ); | |||
|
|
|||
| // wait for child processes to prevent EBUSY | |||
| let _m = FORK_MTX.lock(); | |||
There was a problem hiding this comment.
Is this lock necessary, this test does not open file descriptors, so theoretically no one could access the file system mounted at tempdir
There was a problem hiding this comment.
You are right, the File::create has to fail, so this is not necessary
Co-authored-by: SteveLauC <stevelauc@outlook.com>
fork() duplicates all open file descriptors, which then could prevent a successful umount().
So either when creating file descriptors or when requiring a file descriptor to be closed, we must make sure that no child processes have copies. since wile descriptors are created all over the place, It is more practical to wait for all child processes before unlock.
TODO: