Skip to content

Add suppressed exception details in NativeFSLockFactory lock acquisit…#16303

Open
Bharathi-Kanna wants to merge 4 commits into
apache:mainfrom
Bharathi-Kanna:fix-nativefslockfactory-addsuppressed
Open

Add suppressed exception details in NativeFSLockFactory lock acquisit…#16303
Bharathi-Kanna wants to merge 4 commits into
apache:mainfrom
Bharathi-Kanna:fix-nativefslockfactory-addsuppressed

Conversation

@Bharathi-Kanna

Copy link
Copy Markdown
Contributor

Follow-up to #16278. Resolves the // TODO: addSuppressed in NativeFSLockFactory.tryLock.

Now that the concerns are separated, this replaces the silent swallowing of channel close exceptions on lock failure with IOUtils.closeWhileSuppressingExceptions, attaching any close failure as a suppressed exception.

Changes

  • NativeFSLockFactory.java: Swapped finally/closeWhileHandlingException for catch (Throwable t)/closeWhileSuppressingExceptions in tryLock.
  • TestNativeFSLockFactory.java: Added testSuppressedExceptionOnLockFailure using a mock filesystem to verify the suppressed exception behavior.
  • CHANGES.txt: Added release log entry.

@github-actions github-actions Bot added this to the 11.0.0 milestone Jun 29, 2026
@Bharathi-Kanna

Copy link
Copy Markdown
Contributor Author

Hi @rmuir, this is the follow-up PR to #16278 that resolves the remaining TODO: addSuppressed in tryLock. I'd appreciate your review when you have a chance!

@uschindler uschindler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this new code is fine. Goes in line with other code in Lucene, e.g.,:

try {
return new OrdsBlockTreeTermsWriter(
state, postingsWriter, minTermBlockSize, maxTermBlockSize);
} catch (Throwable t) {
IOUtils.closeWhileSuppressingExceptions(t, postingsWriter);
throw t;
}

The used method ('closeWhileHandlingExceptions()') here was written for that use-case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this variable inside try block, it is no longer needed outside.

@uschindler uschindler requested a review from rmuir June 29, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants