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

File reads aren't thread safe #4

Closed
Tim-Evans-Seequent opened this issue Jun 18, 2024 · 0 comments · Fixed by #7
Closed

File reads aren't thread safe #4

Tim-Evans-Seequent opened this issue Jun 18, 2024 · 0 comments · Fixed by #7
Assignees
Labels
bug Something isn't working

Comments

@Tim-Evans-Seequent
Copy link
Contributor

Tim-Evans-Seequent commented Jun 18, 2024

The OMF code is incorrectly assuming that std::fs::File::try_clone returns an entirely separate file object. It does not: the returned object uses the same file handle and any seek affects all clones. This breaks OMF file reading in multi-threaded environments.

There are a couple of options for fixing this:

  • Implement a file deep copy. This would duplicate the underlying Unix file descriptor or Windows file handle using OS-specific code. I've written this elsewhere and it works.

  • Create a new implementation of std::fs::Read, wrapping Arc<File> and implementing clone that keeps the positions separate. This would again require OS-specific code, as Unix and Windows have slightly different implementations of the required calls: read_at vs. seek_read.

Either of these options could be added to the existing omf::file::sub_file::SubFile type. The second might make more sense there as it's already tracking the file position to implement the sub-file.

@Tim-Evans-Seequent Tim-Evans-Seequent added the bug Something isn't working label Jun 18, 2024
@Tim-Evans-Seequent Tim-Evans-Seequent changed the title Zip access is not thread safe File reads are thread safe Jun 18, 2024
@Tim-Evans-Seequent Tim-Evans-Seequent changed the title File reads are thread safe File reads aren't thread safe Jun 18, 2024
@Tim-Evans-Seequent Tim-Evans-Seequent self-assigned this Jun 18, 2024
andrew-catalyst referenced this issue in seequent/omf-rust Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
1 participant