Make windows filelocking exclusive#3285
Conversation
|
Windows experts review highly welcome here, though the win-CI is green. |
Signed-off-by: apostasie <[email protected]>
| } | ||
| 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".
| // 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? |
3 participants
See #3284
Very likely fix #3228 and possibly a range of other issues...