When corruption strikes, don't create exceptions with circular references #8331
Conversation
| current = corruptedEngine.get(); | ||
| assert current != null; | ||
| current.addSuppressed(e); | ||
| if (current == null) { |
There was a problem hiding this comment.
Is this possible? we have a corruptedEngine.compareAndSet above?
There was a problem hiding this comment.
Yes, the situation, as noted in the comment is this:
e = SomeException
ce = ExceptionsHelper.unwrap(e, CorruptIndexException.class)
when current was 'null' for the CAS operation, we know that either ce == e or that ce is the root cause of e that was unwrapped, so we cannot ce.addSuppressed(e) or it creates a cycle.
|
good catch. Left one comment... |
|
I tried to give a better explanation. after we compareAndSet current, we are just testing if current was previously unset (null). In that case its either e itself or from the initCause hierarchy of e, so we can't add e as a suppressed exception to it. Otherwise, if current was set, then its a separate exception alltogether, so we can safely addSuppressed. |
|
I tried to simplify the code here so its more intuitive: @bleskes is this easier to grok? |
|
LGTM |
|
@rmuir I missed the removal of current=corruptedEngine.get(). The new version is anyway much better. Thx and sorry for the noise. LGTM. |
clintongormley
changed the title
clintongormley
changed the title
…lar references Closes elastic#8331
No description provided.