yuqi1129 commented on code in PR #13249:
URL: https://github.com/apache/gravitino/pull/13249#discussion_r4072942249
##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java:
##########
@@ -301,20 +301,36 @@ static boolean definedBy(@Nullable Object value,
ClassLoader classLoader) {
* <p>All shutdown hooks are run with the system class loader, so we need to
manually clear the
* shutdown hooks registered by the target class loader.
*
+ * <p>The map is JVM-global and {@code
ApplicationShutdownHooks.add/remove/runHooks} synchronize
+ * on the {@code ApplicationShutdownHooks} class monitor, so reading the
field, null-checking it,
+ * and mutating the map must all hold that monitor. {@code runHooks} nulls
the field under the
+ * monitor once shutdown starts, in which case there is nothing left for us
to clear.
+ *
* @param targetClassLoader the classloader where the shutdown hooks are
registered.
*/
- private static void clearShutdownHooks(ClassLoader targetClassLoader) throws
Exception {
+ @VisibleForTesting
+ static void clearShutdownHooks(ClassLoader targetClassLoader) throws
Exception {
Class<?> shutdownHooks =
Class.forName("java.lang.ApplicationShutdownHooks");
- IdentityHashMap<Thread, Thread> hooks =
- (IdentityHashMap<Thread, Thread>)
FieldUtils.readStaticField(shutdownHooks, "hooks", true);
-
- hooks
- .entrySet()
- .removeIf(
- entry -> {
- Thread thread = entry.getKey();
- return thread.getContextClassLoader() == targetClassLoader;
- });
+
+ // Read the field, null-check it, and mutate the map all under the same
monitor the JDK's
+ // add/remove/runHooks hold. During shutdown runHooks nulls the field
while holding this
+ // monitor, so reading it outside the lock could see a stale map or throw
an NPE.
+ synchronized (shutdownHooks) {
+ IdentityHashMap<Thread, Thread> hooks =
+ (IdentityHashMap<Thread, Thread>)
+ FieldUtils.readStaticField(shutdownHooks, "hooks", true);
+ if (hooks == null) {
Review Comment:
When will it be null? Is this just defensive programming?
--
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]
