Fix readonly file mapping on windows #307
Conversation
|
Can someone double-check this on windows? Thanks. I have no access to windows machine unfortunately. |
|
Tried to open a 500mb read-only db and I received an out-of-memory issue. Using this patch and it worked fine. Windows 11 x64, go 1.18.2 |
|
This change looks right, I believe the the big issue from #252 is that the high and low parameters are inverted/swapped. I'll see if I can test it locally but I'm pretty sure that the code in master is never working since those parameters are passed incorrectly. |
| if h == 0 { | ||
| return os.NewSyscallError("CreateFileMapping", errno) | ||
| } | ||
|
|
||
| // Create the memory map. | ||
| addr, errno := syscall.MapViewOfFile(h, syscall.FILE_MAP_READ, 0, 0, uintptr(sz)) | ||
| addr, errno := syscall.MapViewOfFile(h, syscall.FILE_MAP_READ, 0, 0, 0) | ||
| if addr == 0 { | ||
| return os.NewSyscallError("MapViewOfFile", errno) |
There was a problem hiding this comment.
Can leak handle here, see edsrzf/mmap-go@82d537b
There was a problem hiding this comment.
It is unrelated to the PR, but I have added a commit with fix. Uses syscall package instead of windows one, to make it similar with the following code.
|
FWIW, mmap-go also passes https://github.com/edsrzf/mmap-go/blob/main/mmap_windows.go#L56-L59 And the docs for https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createfilemappinga The docs for https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffile
|
`CreateFileMapping` tries to extend the file to the size of mapping which leads to failure if database was opened in readonly and calculated mmap size is bigger than the file size. Providing 0 to `MapViewOfFile` will create a view which has size of mapping, i.e. file-size in read-only mode and full size if file was truncated. Also, swap `sizehi` and `sizelo` names to reflect windows API docs. This was changed in 1c97a49 for seemingly no reason. Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
mmap-go does this, see edsrzf/mmap-go@82d537b Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
|
@fyrchik Please add a changelog item in CHANGELOG.md |
I have encountered a problem where my DB won't open on windows in readonly mode. This seems to be related to #252 as databases there also have bad size.
CreateFileMappingtries to extend the file to the size of mappingwhich leads to failure if database was opened in readonly and calculated
mmap size is bigger than the file size.
Providing 0 to
MapViewOfFilewill create a view which has size ofmapping, i.e. file-size in read-only mode and full size if file was
truncated.
Also, swap
sizehiandsizelonames to reflect windows API docs.This was changed in 1c97a49 for seemingly no reason.
I have tested this on my windows 8 x64 machine, my case has been resolved.
@lunemec, I wonder if this fixes your issue too.