yuqi1129 commented on code in PR #12124:
URL: https://github.com/apache/gravitino/pull/12124#discussion_r3622203068
##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java:
##########
@@ -178,8 +184,25 @@ private static Thread[] getAllThreads() {
return threads;
}
- private static void clearThreadLocalMap(Thread thread, ClassLoader
targetClassLoader) {
- if (thread == null ||
!thread.getName().startsWith("Gravitino-webserver-")) {
+ @VisibleForTesting
+ static void clearThreadLocalMap(Thread thread, ClassLoader
targetClassLoader) {
+ if (thread == null) {
+ return;
+ }
+
+ // Sweep every application thread, not only the Gravitino-webserver-*
ones: a ThreadLocal
+ // pointing at the target ClassLoader can live on any thread, such as a
Caffeine ForkJoinPool
+ // worker, a catalog-cleaner thread, or a Hadoop daemon. The check below
clears only entries
+ // whose value was loaded by the target ClassLoader, so ThreadLocals owned
by other
+ // ClassLoaders are left alone.
+ //
+ // Skip threads whose immediate group is the JVM "system" group (Reference
Handler, Finalizer,
+ // Signal Dispatcher, and the like): they hold no catalog ClassLoader
references, and reflecting
+ // into their ThreadLocals is best avoided. This checks the immediate
group only, so threads in
+ // a sub-group of system (such as InnocuousThreadGroup for common-pool
workers) are still swept.
+ // That is intended: ForkJoinPool threads can hold catalog ThreadLocals.
+ ThreadGroup group = thread.getThreadGroup();
+ if (group != null && "system".equals(group.getName())) {
Review Comment:
Have you tested and verified that it works or not? I'm not sure whether
removing logic `Gravitino-webserver` and replacing it with the thread `group`
can function well when the catalog is closed.
--
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]
