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

Fixed data race about fuse_fill_dir_t function and data pointer #2214

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

ggtakec
Copy link
Member

@ggtakec ggtakec commented Jul 13, 2023

Relevant Issue (if applicable)

#2213 #2212

Details

In the thread callback handler for HeadRequest called by FUSE readdir, void* buf and fuse_fill_dir_t filler seem not suitable for asynchronous writes.
I checked the FUSE source code, but it seems that exclusive control for buf is not implemented by filler.

s3fs HeadRequests are processed in parallel by multithreading, so it seems that this filler call needs to be synchronized.
However, ThreadSanitizer never detected this part yet.
(Even if exclusive control is performed in the filler, I have decided that exclusive control on the s3fs side is not a particular problem, so I am submitting this PR.)

This fix obsoletes multi_head_callback_param uses in threading functions and uses SyncFiller objects instead.
All data related to filler and buf are implemented inside the SyncFiller object.
(This includes filled variables added by #2212)
SyncFiller object handles buf/filler/filled safely.

@ggtakec ggtakec requested a review from gaul July 13, 2023 12:47
@ggtakec ggtakec force-pushed the fix_filler_datarace branch 2 times, most recently from 82e7899 to bb4fc03 Compare July 13, 2023 13:00
This was referenced Jul 13, 2023
@gaul gaul merged commit d0a944f into s3fs-fuse:master Jul 14, 2023
17 checks passed
@ggtakec ggtakec deleted the fix_filler_datarace branch July 16, 2023 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants