From d070ccbc031c70f3476d43e0670f94bc2ddc03cc Mon Sep 17 00:00:00 2001 From: c-blake Date: Wed, 15 Feb 2023 11:41:28 -0500 Subject: [PATCH] Fix `closeHandle` bug, add `setFileSize`, make `resize` work on Windows (#21375) * Add general purpose `setFileSize` (unexported for now). Use to simplify `memfiles.open` as well as make robust (via hard allocation, not merely `ftruncate` address space allocation) on systems with `posix_fallocate`. As part of this, fix a bad `closeHandle` return check bug on Windows and add `MemFile.resize` for Windows now that setFileSize makes that easier. * Adapt existing test to exercise newly portable `MemFile.resize`. * Since Apple has never provided `posix_fallocate`, provide a fallback. This is presently written in terms of `ftruncate`, but it can be improved to use `F_PREALLOCATE` instead, as mentioned in a comment. (cherry picked from commit c91ef1a09f3284315e9e66c32532f5a8e3ba9297) --- lib/posix/posix.nim | 11 ++++-- lib/pure/memfiles.nim | 70 +++++++++++++++++++++++++++++-------- tests/stdlib/tmemfiles2.nim | 5 +-- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim index b1ac4cebcf..1573d0e805 100644 --- a/lib/posix/posix.nim +++ b/lib/posix/posix.nim @@ -198,8 +198,14 @@ proc open*(a1: cstring, a2: cint, mode: Mode | cint = 0.Mode): cint {.inline.} = proc posix_fadvise*(a1: cint, a2, a3: Off, a4: cint): cint {. importc, header: "".} -proc posix_fallocate*(a1: cint, a2, a3: Off): cint {. - importc, header: "".} + +proc ftruncate*(a1: cint, a2: Off): cint {.importc, header: "".} +when defined(osx): # 2001 POSIX evidently does not concern Apple + proc posix_fallocate*(a1: cint, a2, a3: Off): cint = + ftruncate(a1, a2 + a3) # Set size to off + len, max offset +else: # TODO: Use fcntl(fd, F_PREALLOCATE, ..) above + proc posix_fallocate*(a1: cint, a2, a3: Off): cint {. + importc, header: "".} when not defined(haiku) and not defined(openbsd): proc fmtmsg*(a1: int, a2: cstring, a3: cint, @@ -488,7 +494,6 @@ proc fpathconf*(a1, a2: cint): int {.importc, header: "".} proc fsync*(a1: cint): cint {.importc, header: "".} ## synchronize a file's buffer cache to the storage device -proc ftruncate*(a1: cint, a2: Off): cint {.importc, header: "".} proc getcwd*(a1: cstring, a2: int): cstring {.importc, header: "", sideEffect.} proc getuid*(): Uid {.importc, header: "", sideEffect.} ## returns the real user ID of the calling process diff --git a/lib/pure/memfiles.nim b/lib/pure/memfiles.nim index 407a358faa..dc0eccd240 100644 --- a/lib/pure/memfiles.nim +++ b/lib/pure/memfiles.nim @@ -28,6 +28,31 @@ proc newEIO(msg: string): ref IOError = new(result) result.msg = msg +proc setFileSize(fh: FileHandle, newFileSize = -1): OSErrorCode = + ## Set the size of open file pointed to by `fh` to `newFileSize` if != -1. + ## Space is only allocated if that is cheaper than writing to the file. This + ## routine returns the last OSErrorCode found rather than raising to support + ## old rollback/clean-up code style. [ Should maybe move to std/osfiles. ] + if newFileSize == -1: + return + when defined(windows): + var sizeHigh = int32(newFileSize shr 32) + let sizeLow = int32(newFileSize and 0xffffffff) + let status = setFilePointer(fh, sizeLow, addr(sizeHigh), FILE_BEGIN) + let lastErr = osLastError() + if (status == INVALID_SET_FILE_POINTER and lastErr.int32 != NO_ERROR) or + setEndOfFile(fh) == 0: + result = lastErr + else: + var e: cint # posix_fallocate truncates up when needed. + when declared(posix_fallocate): + while (e = posix_fallocate(fh, 0, newFileSize); e == EINTR): + discard + if e in [EINVAL, EOPNOTSUPP] and ftruncate(fh, newFileSize) == -1: + result = osLastError() # fallback arguable; Most portable, but allows SEGV + elif e != 0: + result = osLastError() + type MemFile* = object ## represents a memory mapped file mem*: pointer ## a pointer to the memory mapped file. The pointer @@ -175,17 +200,8 @@ proc open*(filename: string, mode: FileMode = fmRead, if result.fHandle == INVALID_HANDLE_VALUE: fail(osLastError(), "error opening file") - if newFileSize != -1: - var - sizeHigh = int32(newFileSize shr 32) - sizeLow = int32(newFileSize and 0xffffffff) - - var status = setFilePointer(result.fHandle, sizeLow, addr(sizeHigh), - FILE_BEGIN) - let lastErr = osLastError() - if (status == INVALID_SET_FILE_POINTER and lastErr.int32 != NO_ERROR) or - (setEndOfFile(result.fHandle) == 0): - fail(lastErr, "error setting file size") + if (let e = setFileSize(result.fHandle.FileHandle, newFileSize); + e != 0.OSErrorCode): fail(e, "error setting file size") # since the strings are always 'nil', we simply always call # CreateFileMappingW which should be slightly faster anyway: @@ -219,7 +235,7 @@ proc open*(filename: string, mode: FileMode = fmRead, result.wasOpened = true if not allowRemap and result.fHandle != INVALID_HANDLE_VALUE: - if closeHandle(result.fHandle) == 0: + if closeHandle(result.fHandle) != 0: result.fHandle = INVALID_HANDLE_VALUE else: @@ -242,9 +258,8 @@ proc open*(filename: string, mode: FileMode = fmRead, # Is there an exception that wraps it? fail(osLastError(), "error opening file") - if newFileSize != -1: - if ftruncate(result.handle, newFileSize) == -1: - fail(osLastError(), "error setting file size") + if (let e = setFileSize(result.handle.FileHandle, newFileSize); + e != 0.OSErrorCode): fail(e, "error setting file size") if mappedSize != -1: result.size = mappedSize @@ -327,6 +342,31 @@ when defined(posix) or defined(nimdoc): raiseOSError(osLastError()) f.mem = newAddr f.size = newFileSize + else: + raiseOSError(osLastError()) + elif defined(posix): + if f.handle == -1: + raise newException(IOError, + "Cannot resize MemFile opened with allowRemap=false") + if newFileSize != f.size: + if (let e = setFileSize(f.handle.FileHandle, newFileSize); + e != 0.OSErrorCode): raiseOSError(e) + when defined(linux): #Maybe NetBSD, too? + # On Linux this can be over 100 times faster than a munmap,mmap cycle. + proc mremap(old: pointer; oldSize, newSize: csize_t; flags: cint): + pointer {.importc: "mremap", header: "".} + let newAddr = mremap(f.mem, csize_t(f.size), csize_t(newFileSize), 1.cint) + if newAddr == cast[pointer](MAP_FAILED): + raiseOSError(osLastError()) + else: + if munmap(f.mem, f.size) != 0: + raiseOSError(osLastError()) + let newAddr = mmap(nil, newFileSize, PROT_READ or PROT_WRITE, + f.flags, f.handle, 0) + if newAddr == cast[pointer](MAP_FAILED): + raiseOSError(osLastError()) + f.mem = newAddr + f.size = newFileSize proc close*(f: var MemFile) = ## closes the memory mapped file `f`. All changes are written back to the diff --git a/tests/stdlib/tmemfiles2.nim b/tests/stdlib/tmemfiles2.nim index 1b249898ee..2817273432 100644 --- a/tests/stdlib/tmemfiles2.nim +++ b/tests/stdlib/tmemfiles2.nim @@ -12,8 +12,9 @@ var if fileExists(fn): removeFile(fn) -# Create a new file, data all zeros -mm = memfiles.open(fn, mode = fmReadWrite, newFileSize = 20) +# Create a new file, data all zeros, starting at size 10 +mm = memfiles.open(fn, mode = fmReadWrite, newFileSize = 10, allowRemap=true) +mm.resize 20 # resize up to 20 mm.close() # read, change