go-fuse
go-fuse copied to clipboard
Initialize the loops waitGroup when initializing the server
Otherwise, the Serve() and Unmount() calls will race calling loops.Add(1) and loops.Wait() and the outcome is undefined.
Signed-off-by: Shayan Pooya [email protected]
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed, please reply here (e.g. I signed it!
) and we'll verify. Thanks.
- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
- If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
- In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.
your two pull requests seem conflicting: if you are always calling WaitMount, then you would always call Unmount after WaitMount. Since WaitMount waits for Serve to process the first request, there is no race.
The main purpose of this patch is to have a way to correctly schedule a umount to be called after a hard deadline; to unmount and kill any run-away servers. If there is something wrong with the fuse server, the WaitMount() itself might hang. So I don't think it is the best idea to start such a timer after WaitMount() returns.
can you specify more exactly what "something wrong" means? If your server is broken and in an unknown state, then is there any sense in worrying about deadlines?
This is not an issue for the production use-case where the server's state is unknown. We are trying to run a server in a test and we have a timeout to kill the server (and fail the test) if it runs for too long. Using golang's -race option correctly reports this race between calling Serve() and Unmount().
I think the real problem is that the API is designed in the wrong way.
The Serve loop should be started from NewServer. Possibly, it should even do WaitMount implicitly.
Then there should be a Server.Wait() which waits until the serve loop exits.
Yes. But changing the API in that way would be backwards-incompatible I assume. Of course, up to you to decide whether to incorporate this change or not, not a big issue.
Let me see what kind of problems changing the API would create.