Skip to content

Conversation

@DQinYuan
Copy link

#1794 #2137
都提到过 Arthas 存在 stop 后,ArthasClassloader 无法回收的问题。
#2978 中排查的结论是 Arthas 的在退出时没有对 ThreadLocal 进行清理,ArthasClassloader加载的相关对象泄露在了业务线程的 Thread.threadlocals.table 中,最终导致 ArthasClassloader 无法被 gc 回收。

主要是 ThreadLocalWatch.timestampRefExpressFactory.expressRef 两个 threadLocal 未清理导致:

image image

本 PR 的思路就是在 AdviceListener 的 destory 调用清空相关 ThreadLocal。因为 Java 的 ThreadLocal 不支持全部清空,只能清理当前线程,所以基于 ConcurrentHashMap 重新写了一个 CleanableThreadLocal 方便清理。

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an ArthasClassloader memory leak issue caused by uncleaned ThreadLocal variables (ThreadLocalWatch.timestampRef and ExpressFactory.expressRef) that prevented garbage collection after Arthas stopped. The fix introduces a custom CleanableThreadLocal implementation that allows clearing all thread-bound values at once, and ensures cleanup is performed when advice listeners are destroyed.

Key changes:

  • Introduced CleanableThreadLocal class to replace standard ThreadLocal with support for clearing all thread-bound values
  • Updated ThreadLocalWatch and ExpressFactory to use CleanableThreadLocal and expose cleanup methods
  • Added destroy() method overrides in multiple advice listeners to ensure ThreadLocal cleanup on session/command termination

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
CleanableThreadLocal.java New utility class providing ThreadLocal functionality with cleanUp capability using ConcurrentHashMap
ThreadLocalWatch.java Replaced ThreadLocal with CleanableThreadLocal and added cleanUp method
ExpressFactory.java Replaced ThreadLocal with CleanableThreadLocal and added static cleanUp method
WatchAdviceListener.java Added destroy override to clean up ThreadLocal data
TimeTunnelAdviceListener.java Added destroy override to clean up ThreadLocal data
StackAdviceListener.java Added destroy override to clean up ThreadLocal data
MonitorAdviceListener.java Enhanced existing destroy method with ThreadLocal cleanup
AbstractTraceAdviceListener.java Enhanced existing destroy method with ThreadLocal cleanup
TimeTunnelCommand.java Added finally blocks to ensure ExpressFactory cleanup in processWatch and processSearch

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant