oscerd commented on PR #29098: URL: https://github.com/apache/camel/pull/29098#issuecomment-6629542601
Thanks @davsclaus @gnodet — pushed 963ccbd. Taking the split you and I discussed: the three findings that stop the claim working land here, the two that change its ownership model go to **CAMEL-26588**. **Fixed here** 1. **The lock was being dropped.** Verified with a two-process probe before touching anything: closing the second channel in the OFE handler releases this process's lock while `FileLock.isValid()` still answers `true`. Your reading was right; the "modern Linux JDKs use OFD locks so it is safe" line does not hold on this JDK. The channel is no longer closed there. 2. **The claim is atomic.** `computeIfAbsent` in both `PQCProducer` and `FileBasedKeyLifecycleManager`, so the `contains → claim → add` window is gone and the OFE path is only reachable for a genuinely different manager instance. 3. **The stored index is re-read and adopted.** Without it the claim only serialises signers — the arriving instance still resumed from its own startup index. This is the one that made the feature actually do what it says. Plus the two nits: the misplaced `checkStatefulKeyBeforeSign` javadoc, and the redundant `label`. The `.lock` files are deliberately still not deleted, with the reason now in `releaseStatefulKey`: unlinking a file another process holds open and locked lets a third process lock a fresh inode at the same path, giving two owners. **Split to CAMEL-26588** - the claim is released by the first producer to stop when several share a manager (needs a per-key reference count); - it locks the `CamelPQCKeyId` name rather than the signing key, so a different header claims a different file and a missing header skips the claim. **Tests.** Two new ones, both red against the previous commit: - `concurrentFirstClaimsOnOneManagerAllSucceed` — eight threads racing the first claim, previously failing with the misleading "share the KeyLifecycleManager instance"; - `PQCStatefulKeyAdoptionTest` — `expected: <1021> but was: <1023>`, the stored key moving backwards by two. One caveat I want to be straight about: I also added a "third holder is still refused" assertion to `aSecondHolderOfTheSameKeyDirectoryIsRefused`, and it does **not** prove the lock survived — a third holder in the same JVM is refused by `OverlappingFileLockException` before the OS lock is consulted at all, so it passes on the broken code too. I left it in for the registration check and corrected the comment to say what it actually covers. Proving the OS-level behaviour needs a second JVM, which is why I verified it by hand rather than in the suite. Full `camel-pqc` build green: 336 tests, 0 failures. _Claude Code on behalf of @oscerd_ -- 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]
