LuciferYang opened a new pull request, #13249:
URL: https://github.com/apache/gravitino/pull/13249

   ### What changes were proposed in this pull request?
   
   `clearShutdownHooks` now wraps its `removeIf` in `synchronized 
(shutdownHooksClass)`, the same monitor the JDK's `static synchronized` 
`add`/`remove` use. The method becomes `@VisibleForTesting` package-private so 
the locking behavior is directly testable (the public entry short-circuits 
under `GRAVITINO_TEST`, which Gradle sets for all test tasks).
   
   ### Why are the changes needed?
   
   It removed entries from the JVM-global `ApplicationShutdownHooks.hooks` map 
without holding that class's monitor, so a concurrent `Runtime.addShutdownHook` 
could corrupt the map or throw inside the JVM's shutdown machinery.
   
   Fix: #13248
   
   ### Does this PR introduce _any_ user-facing change?
   
   No API change and no functional change on the normal path. 
`clearShutdownHooks` now holds the `ApplicationShutdownHooks` monitor while 
removing hooks, closing a data race with concurrent `addShutdownHook`; the 
method is now `@VisibleForTesting` package-private.
   
   ### How was this patch tested?
   
   Added `TestClassLoaderResourceCleanerUtils`, which pins that, with another 
thread holding the `ApplicationShutdownHooks` monitor, `clearShutdownHooks` 
blocks until the monitor is released while still removing the target hook; on 
the pre-fix tree it completed immediately (no lock).
   


-- 
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]

Reply via email to