XuQianJin-Stars opened a new pull request, #3368:
URL: https://github.com/apache/fluss/pull/3368
## Purpose
`FileLock#tryLock()` previously caught every `Exception` and returned
`false`, which had two issues: (1) genuine I/O failures were indistinguishable
from inter-process lock contention, hiding real problems from callers; and (2)
when lock acquisition failed *after* `init()` had opened the
`FileOutputStream`, that stream was never closed, leaking the underlying file
descriptor for the lifetime of the JVM.
`FileLock#unlockAndDestroy()` also chained `unlock()`, channel close and
stream close inside a single `try` block; if releasing the lock or closing the
channel threw, the `FileOutputStream` was never closed and the lock file might
not be deleted.
This PR fixes both bugs and adds a dedicated test class for `FileLock`.
Linked issue: close #xxx
## Brief change log
- `FileLock#tryLock()`
- Catch `OverlappingFileLockException` explicitly to preserve the
documented "return `false` on contention" semantics.
- Propagate `IOException` / `RuntimeException` so callers can react to
real failures instead of silently treating them as contention.
- Close the `FileOutputStream` opened by `init()` whenever lock
acquisition fails, preventing the file-descriptor leak.
- `FileLock#unlockAndDestroy()`
- Rewritten with nested `try` / `finally` blocks so the channel and the
output stream are each closed independently, and the lock file is always
deleted at the end — even if releasing the lock or closing the channel throws.
- No behavioral change for the happy path; `NetUtilsTest` (the only
production caller) still passes unchanged.
## Tests
Added `FileLockTest` in `fluss-common`, covering:
- happy path: `tryLock()` → `unlock()` → `unlockAndDestroy()` and lock file
is removed.
- in-JVM contention: a second `tryLock()` on the same path returns `false`
and does **not** throw or leak the stream.
- reuse of the same `FileLock` instance after `unlockAndDestroy()`
(re-acquire works).
- `unlockAndDestroy()` is safe to call when `tryLock()` was never invoked.
- `isValid()` returns `false` before any `tryLock()`.
- constructor rejects file names composed entirely of illegal characters.
Existing tests:
- `NetUtilsTest` (the only production user of `FileLock`) continues to pass.
## API and Format
- Public API: **no changes**. Method signatures and contracts of `FileLock`
are preserved; only internal exception handling and resource cleanup are
corrected.
- Wire / serialization format: not affected.
- Configuration: not affected.
## Documentation
- No user-facing documentation changes required.
- Method-level Javadoc on `tryLock()` already states that `false` means the
lock is held elsewhere; the new behavior aligns the implementation with that
contract (real I/O errors are no longer hidden behind `false`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
