Make windows filelocking exclusive #3285
Conversation
|
Windows experts review highly welcome here, though the win-CI is green. |
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
| @@ -31,13 +31,12 @@ func WithDirLock(dir string, fn func() error) error { | |||
| } | |||
| defer dirFile.Close() | |||
| // see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx | |||
| // 1 lock immediately | |||
There was a problem hiding this comment.
This was never "1 = lock immediately" 🤦♂️. It is "1 = The function returns immediately if it is unable to acquire the requested lock. Otherwise, it waits.". We obviously want "wait".
| @@ -31,13 +31,12 @@ func WithDirLock(dir string, fn func() error) error { | |||
| } | |||
| defer dirFile.Close() | |||
| // see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx | |||
| // 1 lock immediately | |||
| if err = windows.LockFileEx(windows.Handle(dirFile.Fd()), 1, 0, 1, 0, &windows.Overlapped{}); err != nil { | |||
| if err = windows.LockFileEx(windows.Handle(dirFile.Fd()), windows.LOCKFILE_EXCLUSIVE_LOCK, 0, ^uint32(0), ^uint32(0), new(windows.Overlapped)); err != nil { | |||
There was a problem hiding this comment.
The docs say:
If an exclusive lock is requested for a range of a file that already has a shared or exclusive lock, the function
returns the error ERROR_IO_PENDING. The system will signal the event specified in the [OVERLAPPED]
(https://learn.microsoft.com/en-us/windows/desktop/api/minwinbase/ns-minwinbase-overlapped) structure
after the lock is granted.
So it might be possible that this fails if something already has the lock and the file was opened with asynchronous i/o. It doesn't appear to be the case here so we should be good with this approach, I mention it just in case something changes in the future.
Thanks a lot for the extra info and the review @jsturtevant . Honestly, I am very close to biting the bullet and writing tests for this stuff. I wish we had something usable off the bat instead of having to maintain that ( https://github.com/golang/proposal/blob/master/design/33974-add-public-lockedfile-pkg.md ). |
|
@AkihiroSuda do you think we can get that in? |
See #3284
Very likely fix #3228 and possibly a range of other issues...