From 00d7f7f733d0f0b0c47ffcef73bf4c660e3a7e2e Mon Sep 17 00:00:00 2001 From: Trung Nguyen <57174311+trungnt2910@users.noreply.github.com> Date: Thu, 20 Apr 2023 17:04:11 +1000 Subject: [PATCH] fix(monika): _kern_close double close Fixed a bug when `_kern_close` translates into two host `close` syscalls when the file descriptor refers to a directory. This bug appear harmless for single-threaded programs but affects programs like `launch_daemon` which opens a directory on both the launcher thread and the main worker thread. The data race of one thread closing a fd a second time right after the other thread opening a new fd makes those programs think that there was actually an error with the new fd. After this fix `launch_daemon` should reliably launch on `hyclone_server` startup. There should be no more "id: cannot find name for user ID 0" errors on HyClone. --- haiku_loader/loader_readdir.h | 2 +- haiku_loader/sys/linux/loader_readdir.cpp | 4 ++-- monika/linux/io.cpp | 12 +++++++----- shared_headers/extended_commpage.h | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/haiku_loader/loader_readdir.h b/haiku_loader/loader_readdir.h index 8636ce1..e40151f 100644 --- a/haiku_loader/loader_readdir.h +++ b/haiku_loader/loader_readdir.h @@ -9,7 +9,7 @@ // a directory fd may cause directory entries to be left behind. void loader_opendir(int fd); -void loader_closedir(int fd); +bool loader_closedir(int fd); int loader_readdir(int fd, void* buffer, size_t bufferSize, int maxCount); void loader_rewinddir(int fd); void loader_dupdir(int oldFd, int newFd); diff --git a/haiku_loader/sys/linux/loader_readdir.cpp b/haiku_loader/sys/linux/loader_readdir.cpp index 4e46c06..1490ac7 100644 --- a/haiku_loader/sys/linux/loader_readdir.cpp +++ b/haiku_loader/sys/linux/loader_readdir.cpp @@ -67,10 +67,10 @@ void loader_opendir(int fd) loader_opendir_unlocked(fd); } -void loader_closedir(int fd) +bool loader_closedir(int fd) { std::unique_lock lock(sFdMapMutex); - sFdMap.erase(fd); + return sFdMap.erase(fd); } void loader_dupdir(int oldFd, int newFd) diff --git a/monika/linux/io.cpp b/monika/linux/io.cpp index fa65401..007b108 100644 --- a/monika/linux/io.cpp +++ b/monika/linux/io.cpp @@ -548,14 +548,16 @@ int _moni_close(int fd) return HAIKU_POSIX_EBADF; } - int result = LINUX_SYSCALL1(__NR_close, fd); - - if (result < 0) + if (!GET_HOSTCALLS()->closedir(fd)) { - return LinuxToB(-result); + int result = LINUX_SYSCALL1(__NR_close, fd); + + if (result < 0) + { + return LinuxToB(-result); + } } - GET_HOSTCALLS()->closedir(fd); GET_SERVERCALLS()->unregister_fd(fd); return B_OK; diff --git a/shared_headers/extended_commpage.h b/shared_headers/extended_commpage.h index 5ad0aa0..b94b2fe 100644 --- a/shared_headers/extended_commpage.h +++ b/shared_headers/extended_commpage.h @@ -85,7 +85,7 @@ struct hostcalls // Readdir void (*opendir)(int fd); - void (*closedir)(int fd); + bool (*closedir)(int fd); int (*readdir)(int fd, void* buffer, size_t bufferSize, int maxCount); void (*rewinddir)(int fd); void (*dupdir)(int oldFd, int newFd);