Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

Remove finalize from RecordEventsSpanImpl #2043

Merged
merged 1 commit into from Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion buildscripts/checkstyle.xml
Expand Up @@ -173,7 +173,6 @@
<message key="name.invalidPattern"
value="Interface type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="NoFinalizer"/>
<module name="GenericWhitespace">
<message key="ws.followed"
value="GenericWhitespace ''{0}'' is followed by whitespace."/>
Expand Down
Expand Up @@ -586,15 +586,4 @@ private RecordEventsSpanImpl(
timestampConverter != null ? timestampConverter : TimestampConverter.now(clock);
startNanoTime = clock.nowNanos();
}

@SuppressWarnings("NoFinalizer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please see original PR #1877 which fixed the issue #1876 by adding this code.
As you can see, this code change was deliberate, so removing it would probably require addressing the issue differently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the severity of the issue this code is causing, we decided to revisit the decision and proceed with this change. See #2045 for more context.

@Override
protected void finalize() throws Throwable {
synchronized (this) {
if (!hasBeenEnded) {
logger.log(Level.SEVERE, "Span " + name + " is GC'ed without being ended.");
}
}
super.finalize();
}
}