From 3914749f5694a7f1f938d07e303a8be61e227e94 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 25 Mar 2026 16:42:22 +0300 Subject: [PATCH] os.filelock: fix unreliable locking for existing files (fixes #12059) --- vlib/os/filelock/filelock_helpers.h | 63 ++++++++++++ vlib/os/filelock/filelock_test.v | 151 +++++++++++++++++++++++++++- vlib/os/filelock/lib.v | 49 +++++++-- vlib/os/filelock/lib_nix.c.v | 94 +++++++++++------ vlib/os/filelock/lib_windows.c.v | 92 +++++++++++++---- 5 files changed, 390 insertions(+), 59 deletions(-) create mode 100644 vlib/os/filelock/filelock_helpers.h diff --git a/vlib/os/filelock/filelock_helpers.h b/vlib/os/filelock/filelock_helpers.h new file mode 100644 index 000000000..50cdb5a70 --- /dev/null +++ b/vlib/os/filelock/filelock_helpers.h @@ -0,0 +1,63 @@ +#ifndef V_OS_FILELOCK_HELPERS_H +#define V_OS_FILELOCK_HELPERS_H + +#include + +#ifdef _WIN32 +#include + +static int v_filelock_lock(HANDLE handle, int exclusive, int immediate, + unsigned long long start, unsigned long long len) { + OVERLAPPED overlap; + memset(&overlap, 0, sizeof(overlap)); + overlap.Offset = (DWORD)(start & 0xffffffffULL); + overlap.OffsetHigh = (DWORD)(start >> 32); + DWORD flags = immediate ? LOCKFILE_FAIL_IMMEDIATELY : 0; + if (exclusive) { + flags |= LOCKFILE_EXCLUSIVE_LOCK; + } + DWORD low = len == 0 ? MAXDWORD : (DWORD)(len & 0xffffffffULL); + DWORD high = len == 0 ? MAXDWORD : (DWORD)(len >> 32); + return LockFileEx(handle, flags, 0, low, high, &overlap) ? 0 : -1; +} + +static int v_filelock_unlock(HANDLE handle, unsigned long long start, + unsigned long long len) { + OVERLAPPED overlap; + memset(&overlap, 0, sizeof(overlap)); + overlap.Offset = (DWORD)(start & 0xffffffffULL); + overlap.OffsetHigh = (DWORD)(start >> 32); + DWORD low = len == 0 ? MAXDWORD : (DWORD)(len & 0xffffffffULL); + DWORD high = len == 0 ? MAXDWORD : (DWORD)(len >> 32); + return UnlockFileEx(handle, 0, low, high, &overlap) ? 0 : -1; +} + +#else +#include +#include + +static int v_filelock_lock(int fd, int exclusive, int immediate, + unsigned long long start, unsigned long long len) { + struct flock fl; + memset(&fl, 0, sizeof(fl)); + fl.l_type = exclusive ? F_WRLCK : F_RDLCK; + fl.l_whence = SEEK_SET; + fl.l_start = (off_t)start; + fl.l_len = len == 0 ? 0 : (off_t)len; + return fcntl(fd, immediate ? F_SETLK : F_SETLKW, &fl); +} + +static int v_filelock_unlock(int fd, unsigned long long start, + unsigned long long len) { + struct flock fl; + memset(&fl, 0, sizeof(fl)); + fl.l_type = F_UNLCK; + fl.l_whence = SEEK_SET; + fl.l_start = (off_t)start; + fl.l_len = len == 0 ? 0 : (off_t)len; + return fcntl(fd, F_SETLK, &fl); +} + +#endif + +#endif diff --git a/vlib/os/filelock/filelock_test.v b/vlib/os/filelock/filelock_test.v index 3420cfbcf..3599923bb 100644 --- a/vlib/os/filelock/filelock_test.v +++ b/vlib/os/filelock/filelock_test.v @@ -1,24 +1,126 @@ import os import os.filelock +import time -fn lockfile_path(name string) string { - return os.join_path(os.vtmp_dir(), 'filelock_test_${os.getpid()}_${name}') +const test_dir = os.join_path(os.vtmp_dir(), 'filelock_tests') + +const helper_mode_env = 'V_FILELOCK_HELPER_MODE' +const helper_path_env = 'V_FILELOCK_HELPER_PATH' +const helper_lock_mode_env = 'V_FILELOCK_LOCK_MODE' +const helper_start_env = 'V_FILELOCK_START' +const helper_len_env = 'V_FILELOCK_LEN' + +fn testsuite_begin() { + if os.getenv(helper_mode_env) != '' { + run_helper_process() + } + os.mkdir_all(test_dir) or {} +} + +fn testsuite_end() { + os.rmdir_all(test_dir) or {} +} + +fn run_helper_process() { + path := os.getenv(helper_path_env) + lock_mode := if os.getenv(helper_lock_mode_env) == 'shared' { + filelock.LockMode.shared + } else { + filelock.LockMode.exclusive + } + start := os.getenv(helper_start_env).u64() + len := os.getenv(helper_len_env).u64() + mut file_lock := filelock.new_file(path, filelock.LockOptions{ + mode: lock_mode + start: start + len: len + }) + match os.getenv(helper_mode_env) { + 'hold' { + file_lock.acquire() or { + eprintln(err.msg()) + exit(2) + } + println('locked') + os.flush() + time.sleep(300 * time.millisecond) + file_lock.release() + exit(0) + } + 'try' { + exit(if file_lock.try_acquire() { + file_lock.release() + 0 + } else { + 1 + }) + } + else { + exit(3) + } + } +} + +fn helper_path(name string) string { + return os.join_path(test_dir, name) +} + +fn new_helper_process(path string, lock_mode filelock.LockMode, start u64, len u64, mode string) &os.Process { + mut p := os.new_process(os.executable()) + mut env := os.environ() + env[helper_mode_env] = mode + env[helper_path_env] = path + env[helper_lock_mode_env] = lock_mode.str() + env[helper_start_env] = start.str() + env[helper_len_env] = len.str() + p.set_environment(env) + p.set_redirect_stdio() + return p +} + +fn wait_for_lock(mut p os.Process) { + mut output := '' + for _ in 0 .. 50 { + if p.is_pending(.stdout) { + output += p.stdout_read() + if output.contains('locked') { + return + } + } + if !p.is_alive() { + break + } + time.sleep(10 * time.millisecond) + } + p.wait() + stderr := p.stderr_slurp().trim_space() + assert false, 'helper failed to acquire lock; exit=${p.code} stdout="${output.trim_space()}" stderr="${stderr}"' +} + +fn try_lock_from_helper(path string, lock_mode filelock.LockMode, start u64, len u64) int { + mut p := new_helper_process(path, lock_mode, start, len, 'try') + p.run() + p.wait() + stderr := p.stderr_slurp().trim_space() + code := p.code + p.close() + assert stderr == '' + return code } fn test_flock() { - lockfile := lockfile_path('test.lock') + lockfile := helper_path('test.lock') os.rm(lockfile) or {} mut l := filelock.new(lockfile) assert !os.exists(lockfile) l.acquire() or { panic(err) } assert os.exists(lockfile) - // do stuff l.release() assert !os.exists(lockfile) } fn test_flock_try() { - lockfile := lockfile_path('test-try.lock') + lockfile := helper_path('test-try.lock') os.rm(lockfile) or {} mut l := filelock.new(lockfile) assert l.try_acquire() @@ -31,3 +133,42 @@ fn test_flock_try() { l.release() assert !os.exists(lockfile) } + +fn test_existing_file_lock_range() { + target := helper_path('existing-range.txt') + os.write_file(target, '0123456789abcdef')! + mut holder := new_helper_process(target, .exclusive, 0, 5, 'hold') + holder.run() + defer { + if holder.status == .running || holder.is_alive() { + holder.wait() + } + holder.close() + } + wait_for_lock(mut holder) + assert try_lock_from_helper(target, .exclusive, 0, 5) == 1 + assert try_lock_from_helper(target, .exclusive, 6, 5) == 0 + holder.wait() + assert holder.code == 0 + assert os.exists(target) + assert os.read_file(target)! == '0123456789abcdef' +} + +fn test_existing_file_lock_shared_mode() { + target := helper_path('existing-shared.txt') + os.write_file(target, '0123456789abcdef')! + mut holder := new_helper_process(target, .shared, 0, 5, 'hold') + holder.run() + defer { + if holder.status == .running || holder.is_alive() { + holder.wait() + } + holder.close() + } + wait_for_lock(mut holder) + assert try_lock_from_helper(target, .shared, 0, 5) == 0 + assert try_lock_from_helper(target, .exclusive, 0, 5) == 1 + holder.wait() + assert holder.code == 0 + assert os.exists(target) +} diff --git a/vlib/os/filelock/lib.v b/vlib/os/filelock/lib.v index cb9d9e24b..ca2d368a1 100644 --- a/vlib/os/filelock/lib.v +++ b/vlib/os/filelock/lib.v @@ -2,19 +2,57 @@ module filelock import time +// LockMode describes whether an existing file lock is shared/read or exclusive/write. +pub enum LockMode { + shared + exclusive +} + +// LockOptions configures how `new_file` locks an existing file. +@[params] +pub struct LockOptions { +pub: + mode LockMode = .exclusive + start u64 + len u64 +} + +enum LockTarget { + sidecar + file +} + pub struct FileLock { - name string + name string + mode LockMode = .exclusive + start u64 + len u64 + target LockTarget = .sidecar mut: fd i64 } -pub fn new(fileName string) FileLock { +// new creates a sidecar lock file that is removed again when the lock is released. +pub fn new(file_name string) FileLock { return FileLock{ - name: fileName + name: file_name fd: -1 } } +// new_file creates a lock for an existing file path using OS-level file locking. +pub fn new_file(path string, options LockOptions) FileLock { + return FileLock{ + name: path + mode: options.mode + start: options.start + len: options.len + target: .file + fd: -1 + } +} + +// wait_acquire keeps trying to acquire the lock until `timeout` expires. pub fn (mut l FileLock) wait_acquire(timeout time.Duration) bool { fin := time.now().add(timeout) for time.now() < fin { @@ -26,11 +64,10 @@ pub fn (mut l FileLock) wait_acquire(timeout time.Duration) bool { return false } +// release unlocks the file and closes the underlying file descriptor or handle. pub fn (mut l FileLock) release() bool { if l.fd != -1 { - unsafe { - l.unlink() - } + l.close_lock() return true } return false diff --git a/vlib/os/filelock/lib_nix.c.v b/vlib/os/filelock/lib_nix.c.v index 78f9271e2..054af1312 100644 --- a/vlib/os/filelock/lib_nix.c.v +++ b/vlib/os/filelock/lib_nix.c.v @@ -1,57 +1,93 @@ module filelock #include +#insert "@VEXEROOT/vlib/os/filelock/filelock_helpers.h" fn C.unlink(&char) i32 fn C.open(&char, i32, i32) i32 +fn C.close(i32) i32 fn C.flock(i32, i32) i32 +fn C.v_filelock_lock(i32, i32, i32, u64, u64) i32 +fn C.v_filelock_unlock(i32, u64, u64) i32 -@[unsafe] -pub fn (mut l FileLock) unlink() { - if l.fd != -1 { - C.close(l.fd) - l.fd = -1 +fn (l &FileLock) open_lock() int { + if l.target == .file { + return open_existing_file(l.name, l.mode) + } + return open_lockfile(l.name) +} + +fn open_lockfile(path string) int { + mut fd := C.open(&char(path.str), C.O_CREAT | C.O_RDONLY, 0o644) + if fd == -1 { + fd = C.open(&char(path.str), C.O_RDONLY, 0) } - C.unlink(&char(l.name.str)) + return fd +} + +fn open_existing_file(path string, mode LockMode) int { + flags := if mode == .shared { C.O_RDONLY } else { C.O_RDWR } + return C.open(&char(path.str), flags, 0) } +fn (l &FileLock) lock_fd(fd int, immediate bool) bool { + if l.target == .file { + return C.v_filelock_lock(fd, int(l.mode == .exclusive), int(immediate), l.start, + l.len) == 0 + } + flags := if immediate { C.LOCK_EX | C.LOCK_NB } else { C.LOCK_EX } + return C.flock(fd, flags) == 0 +} + +fn (mut l FileLock) close_lock() { + if l.fd == -1 { + return + } + fd := int(l.fd) + if l.target == .file { + C.v_filelock_unlock(fd, l.start, l.len) + C.close(fd) + } else { + C.close(fd) + C.unlink(&char(l.name.str)) + } + l.fd = -1 +} + +// acquire blocks until the lock is acquired. pub fn (mut l FileLock) acquire() ! { if l.fd != -1 { return error_with_code('lock already acquired by this instance', 1) } - fd := open_lockfile(l.name) + fd := l.open_lock() if fd == -1 { - return error_with_code('cannot create lock file ${l.name}', -1) + msg := if l.target == .file { + 'cannot open file ${l.name}' + } else { + 'cannot create lock file ${l.name}' + } + return error_with_code(msg, -1) } - if C.flock(fd, C.LOCK_EX) == -1 { + if !l.lock_fd(fd, false) { C.close(fd) - return error_with_code('cannot lock', -2) + return error_with_code('cannot lock ${l.name}', -2) } l.fd = fd } -fn open_lockfile(f string) int { - mut fd := C.open(&char(f.str), C.O_CREAT, 0o644) - if fd == -1 { - // if stat is too old delete lockfile - fd = C.open(&char(f.str), C.O_RDONLY, 0) - } - return fd -} - +// try_acquire tries to acquire the lock without blocking. pub fn (mut l FileLock) try_acquire() bool { if l.fd != -1 { return true } - fd := open_lockfile('${l.name}') - if fd != -1 { - err := C.flock(fd, C.LOCK_EX | C.LOCK_NB) - if err == -1 { - C.close(fd) - return false - } - l.fd = fd - return true + fd := l.open_lock() + if fd == -1 { + return false } - return false + if !l.lock_fd(fd, true) { + C.close(fd) + return false + } + l.fd = fd + return true } diff --git a/vlib/os/filelock/lib_windows.c.v b/vlib/os/filelock/lib_windows.c.v index 004b55763..70758d5c2 100644 --- a/vlib/os/filelock/lib_windows.c.v +++ b/vlib/os/filelock/lib_windows.c.v @@ -1,46 +1,100 @@ module filelock +#insert "@VEXEROOT/vlib/os/filelock/filelock_helpers.h" + fn C.DeleteFileW(&u16) bool fn C.CreateFileW(&u16, u32, u32, voidptr, u32, u32, voidptr) voidptr fn C.CloseHandle(voidptr) bool +fn C.v_filelock_lock(voidptr, int, int, u64, u64) int +fn C.v_filelock_unlock(voidptr, u64, u64) int -pub fn (mut l FileLock) unlink() { - if l.fd != -1 { - C.CloseHandle(voidptr(l.fd)) - l.fd = -1 +fn (l &FileLock) open_lock() i64 { + if l.target == .file { + return open_existing_file(l.name, l.mode) + } + return open_lockfile(l.name) +} + +fn open_lockfile(path string) i64 { + path_wide := path.to_wide() + handle := C.CreateFileW(path_wide, C.GENERIC_READ | C.GENERIC_WRITE, 0, 0, C.OPEN_ALWAYS, + C.FILE_ATTRIBUTE_NORMAL, 0) + return file_handle(handle) +} + +fn open_existing_file(path string, mode LockMode) i64 { + path_wide := path.to_wide() + access := if mode == .shared { C.GENERIC_READ } else { C.GENERIC_READ | C.GENERIC_WRITE } + share_mode := C.FILE_SHARE_READ | C.FILE_SHARE_WRITE | C.FILE_SHARE_DELETE + handle := C.CreateFileW(path_wide, access, share_mode, 0, C.OPEN_EXISTING, C.FILE_ATTRIBUTE_NORMAL, + 0) + return file_handle(handle) +} + +fn file_handle(handle voidptr) i64 { + return if handle == voidptr(-1) { -1 } else { i64(handle) } +} + +fn (l &FileLock) lock_handle(handle voidptr, immediate bool) bool { + if l.target != .file { + return true } - t_wide := l.name.to_wide() - C.DeleteFileW(t_wide) + return C.v_filelock_lock(handle, int(l.mode == .exclusive), int(immediate), l.start, + l.len) == 0 } +fn (mut l FileLock) close_lock() { + if l.fd == -1 { + return + } + handle := voidptr(l.fd) + if l.target == .file { + C.v_filelock_unlock(handle, l.start, l.len) + C.CloseHandle(handle) + } else { + C.CloseHandle(handle) + path_wide := l.name.to_wide() + C.DeleteFileW(path_wide) + } + l.fd = -1 +} + +// acquire blocks until the lock is acquired. pub fn (mut l FileLock) acquire() ! { if l.fd != -1 { return error_with_code('lock already acquired by this instance', 1) } - fd := open(l.name) + fd := l.open_lock() if fd == -1 { - return error_with_code('cannot create lock file ${l.name}', -1) + msg := if l.target == .file { + 'cannot open file ${l.name}' + } else { + 'cannot create lock file ${l.name}' + } + return error_with_code(msg, -1) + } + handle := voidptr(fd) + if !l.lock_handle(handle, false) { + C.CloseHandle(handle) + return error_with_code('cannot lock ${l.name}', -2) } l.fd = fd } -fn open(f string) i64 { - f_wide := f.to_wide() - // locking it - fd := C.CreateFileW(f_wide, C.GENERIC_READ | C.GENERIC_WRITE, 0, 0, C.OPEN_ALWAYS, - C.FILE_ATTRIBUTE_NORMAL, 0) - return i64(fd) -} - +// try_acquire tries to acquire the lock without blocking. pub fn (mut l FileLock) try_acquire() bool { if l.fd != -1 { - // lock already acquired by this instance - return false + return true } - fd := open(l.name) + fd := l.open_lock() if fd == -1 { return false } + handle := voidptr(fd) + if !l.lock_handle(handle, true) { + C.CloseHandle(handle) + return false + } l.fd = fd return true } -- 2.39.5