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

removeMount should lock #17144

Closed
PVince81 opened this issue Jun 24, 2015 · 11 comments · Fixed by #23545
Closed

removeMount should lock #17144

PVince81 opened this issue Jun 24, 2015 · 11 comments · Fixed by #23545

Comments

@PVince81
Copy link
Contributor

@icewind1991 looks like "$view->removeMount()" doesn't lock.

We should exclusive-lock the mount point path (not the storage root) to avoid concurrent processes to access that path during the umount (in case sharing takes a while...)

Requires #17070 to be merged first for the extra argument.

The same should be done for adding mount points (when sharing, etc)

@PVince81 PVince81 added this to the 8.1-current milestone Jun 24, 2015
@PVince81 PVince81 modified the milestones: 8.1.1-next-maintenance, 8.1-current Jul 1, 2015
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 1, 2015

@cmonteroluque @karlitschek @DeepDiver1975 moving to 8.1.1.
Needed to make locking more watertight.

@MorrisJobke
Copy link
Contributor

@PVince81 Is there any PR for this?

@PVince81
Copy link
Contributor Author

No

@DeepDiver1975
Copy link
Member

@PVince81 what is the criticality of this?

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 17, 2015 via email

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.1.2-next-maintenance, 8.1.1-current-maintenance Jul 17, 2015
@ghost ghost modified the milestones: 8.1.3-next-maintenance, 8.1.2-current-maintenance Aug 25, 2015
@ghost
Copy link

ghost commented Aug 25, 2015

@PVince81 agree that it needs to be better, 8.1.2 not looking possible at this point.

@MorrisJobke MorrisJobke modified the milestones: 8.1.3, 8.1.4-current-maintenance Sep 13, 2015
@PVince81
Copy link
Contributor Author

Not sure why we put this on 8.1.4. Other remaining locking tasks like #17243 are now set to 9.0.

@ghost ghost modified the milestones: 9.0-next, 8.1.4-current-maintenance Oct 14, 2015
@icewind1991
Copy link
Contributor

I'm not sure this is needed, I don't think there is any chance for data loss since the worst that can happen is a different process writing to a mount that is being removed, which is the same result as when the other processed finished it's write before the storage is removed

@PVince81
Copy link
Contributor Author

Best would be to do a test with sleep() in the right places to make sure it works as you think it does.

@ghost ghost assigned icewind1991 Feb 23, 2016
@ghost ghost modified the milestones: 9.0.1-next-maintenance, 9.0-current Feb 23, 2016
@PVince81
Copy link
Contributor Author

PVince81 commented Mar 9, 2016

@nickvergessen mind taking over ? If we find that we don't need a lock there then I'm fine dropping it, but we need to verify this.

@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants