Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkdtemp doesn't work with chdir and relative paths #1477

Open
erikcorry opened this issue Mar 9, 2023 · 1 comment
Open

mkdtemp doesn't work with chdir and relative paths #1477

erikcorry opened this issue Mar 9, 2023 · 1 comment
Assignees

Comments

@erikcorry
Copy link
Member

erikcorry commented Mar 9, 2023

There's no -at version of mkdtemp like (openat is a version of open with an explicit 'current directory').

Context: Because there are multiple Toit processes in one Unix process, we never use the chdir syscall. Instead, every Toit process has a handle open on its current directory. We don't use the open syscall, which uses the Unix-process-wide current directory, but instead use openat, which takes an open file descriptor (directory descriptor, actually) as the location of relative paths.

Possible fixes

  • Use the same technique as Windows: Keeping track of the textual current directory for the process and prepending it.
    • This sort of works, but unlike the rest of current-dir support on Unix it fails if the current directory is renamed at a bad moment.
  • Removing support for temporary directories outside of /tmp
    • it's not very useful anyway, I think.
  • Introduce a C++-level mkdtemp mutex and fchdir the whole Unix process (all Toit processes) while doing the mkdtemp.
    • this would probably smoke out any bugs where we accidentally still use the Unix-process-wide current directory on Unix.
@erikcorry erikcorry self-assigned this Mar 9, 2023
@floitsch
Copy link
Member

floitsch commented Mar 9, 2023

We use temporary directories outside of /tmp quite a lot: whenever we need to cache something, we create a temporary directory in the cache. This way we can use an atomic move operation to store the downloaded object in the cache.
If we always used /tmp we could be on a different drive, which would result in a slower and non-atomic copy operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants