On Tue, 12 Jul 2022 18:38:33 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> src/hotspot/os/posix/perfMemory_posix.cpp line 807: >> >>> 805: // Hold the lock until here to prevent other JVMs from using this >>> file >>> 806: // while we are in the middle of deleting it. >>> 807: ::close(fd); >> >> I don't understand this comment, it seems to contradict what you are doing. >> We are closing the only fd referring to this lock file, right? So the lock >> should get unlocked here too? If we want to keep the lock open, shouldn't we >> avoid closing the fd? > > I want to prevent the following race condition. Let's assume this process has > PID 10 and there's another process (in a different pid namespace) with PID > 20. Both process see a file named "20". > > 0. No one holds a lock on this file. > 1. Process 20 successfully locks the file in cleanup_sharedmem_files(). > 2. Process 20 gives up the lock. > 3. Process 20 decides it can delete the file (PID 20 matches its own PID). > 4. This process successfully locks the file in cleanup_sharedmem_files(). > 5. This process gives up the lock > 6. This process decides it can delete the file (PID 20 does not exist in my > pid namespace) > 7. Process 20 deletes the file. Creates a new version of this file. > Successfully locks the new file. > 8. This process deletes the new version of this file (by name). > > By holding the lock between steps 4 and 8, we can guaranteed that if a > process can successfully lock the file in create_sharedmem_file(), this file > will never be unintentionally deleted. > > I changed the comment slightly: > > > // Hold the lock until here to prevent other JVMs from using this file > - // while we are in the middle of deleting it. > + // while we were in the middle of deleting it.
Makes sense, thanks for explaining, and the past tense in the comment helps. ------------- PR: https://git.openjdk.org/jdk/pull/9406
