dubbo icon indicating copy to clipboard operation
dubbo copied to clipboard

AbstractCacheManager destroy Framework's executorService when shutdown

Open huangqy17 opened this issue 1 year ago • 8 comments

The executorService in AbstractCacheManager is passed in from outside and belongs to FrameworkModel. However, when an ApplicationModel is destroyed, the destroy of AbstractCacheManager will be triggered, and the public executorService will be shut down, causing other ApplicationModels to no longer be able to refresh the dubbo cache.


AbstractCacheManager中的executorService是外部传入,属于FrameworkModel,但是一个ApplicationModel销毁时,会触发AbstractCacheManager的destroy,就把这个公共的executorService shutdown 了,导致其他ApplicationModel不能再刷新dubbo cache

huangqy17 avatar Jan 05 '24 06:01 huangqy17

3.1.x和3.2.x版本都存在这个问题

huangqy17 avatar Jan 05 '24 06:01 huangqy17

Can you please submit a PR to fix it? It is needed to check if executorService is being created by org/apache/dubbo/metadata/AbstractCacheManager.java:72.

AlbumenJ avatar Jan 05 '24 09:01 AlbumenJ

Can I work on this? @AlbumenJ

SoulPancake avatar Jan 05 '24 18:01 SoulPancake

Let's add a reference counter to track how many ApplicationModels are using it. We'll increase the count when an ApplicationModel starts and decrease it when one is destroyed. Then, in the destroy method, we only shut down executorService if the counter is at zero, ensuring it stays active while still needed. @huangqy17 @AlbumenJ

SoulPancake avatar Jan 05 '24 18:01 SoulPancake

Let's add a reference counter to track how many ApplicationModels are using it. We'll increase the count when an ApplicationModel starts and decrease it when one is destroyed. Then, in the destroy method, we only shut down executorService if the counter is at zero, ensuring it stays active while still needed. @huangqy17 @AlbumenJ

This will not work, if the count is 0 to shutdown, then I start an applicationModel, again passed the frameworkmodel's executorservice, found to be terminated, still can't perform the cache refresh

huangqy17 avatar Jan 08 '24 02:01 huangqy17

Let's add a reference counter to track how many ApplicationModels are using it. We'll increase the count when an ApplicationModel starts and decrease it when one is destroyed. Then, in the destroy method, we only shut down executorService if the counter is at zero, ensuring it stays active while still needed. @huangqy17 @AlbumenJ

这样不行,如果靠计数为0就shutdown,后面我又启了一个applicationModel,再次传入这个frameworkmodel的executorservice,发现是terminated,仍是执行不了刷新cache

  1. Use a flag to indicate whether the ExecutorService is created by AbstractCacheManager.
  • This approach is straightforward but not elegant.
  1. Inject applicationModel into AbstractCacheManager and compare the executorService reference during destruction.
  • It's uncertain whether AbstractCacheManager will require applicationModel in the future.
  1. Use frameworkmodel::executorservice and add a reference counter; perhaps the ExecutorService created by AbstractCacheManager is unnecessary.

The above are my suggestions, but they may not be correct

Do other executors in FrameworkExecutorRepository encounter a similar question?

  1. 使用flag标记是否由AbstractCacheManager创建
  • 简单但是不优雅
  1. 把applicationModel注入进AbstractCacheManager,删除的时候比较引用
  • 不确定applicationModel未来版本是否能用到
  1. 统一使用frameworkmodel的线程池+上引用计数,保证frameworkmodel线程池创建在前,也许不会有空指针的问题

以上是一点小建议,不一定正确

其他FrameworkExecutorRepository中的线程池不知道是否也有这个问题 @huangqy17

Nortyr avatar Jan 08 '24 03:01 Nortyr

Let's add a reference counter to track how many ApplicationModels are using it. We'll increase the count when an ApplicationModel starts and decrease it when one is destroyed. Then, in the destroy method, we only shut down executorService if the counter is at zero, ensuring it stays active while still needed. @huangqy17 @AlbumenJ

这样不行,如果靠计数为0就shutdown,后面我又启了一个applicationModel,再次传入这个frameworkmodel的executorservice,发现是terminated,仍是执行不了刷新cache

1. Use a flag to indicate whether the ExecutorService is created by AbstractCacheManager.


* This approach is straightforward but not elegant.


2. Inject applicationModel into AbstractCacheManager and compare the executorService reference during destruction.


* It's uncertain whether AbstractCacheManager will require applicationModel  in the future.


3. Use frameworkmodel::executorservice and add a reference counter; perhaps the ExecutorService created by AbstractCacheManager is unnecessary.

The above are my suggestions, but they may not be correct

Do other executors in FrameworkExecutorRepository encounter a similar question?

1. 使用flag标记是否由AbstractCacheManager创建


* 简单但是不优雅


2. 把applicationModel注入进AbstractCacheManager,删除的时候比较引用


* 不确定applicationModel未来版本是否能用到


3. 统一使用frameworkmodel的线程池+上引用计数,保证frameworkmodel线程池创建在前,也许不会有空指针的问题

以上是一点小建议,不一定正确

其他FrameworkExecutorRepository中的线程池不知道是否也有这个问题 @huangqy17

fix in PR #14091, introduce disposable-registration mechanism which is elegant:

  1. when initialize - wrap new created Cache, CacheStore as Disposable and registerDisposable().
  2. when initialize - if parameter executorService is NULL, then construct a new ExecutorService instance and registerDisposable(); otherwise treat the new ScheduledFuture as disposable.
  3. destroy registered disposable resource in FILO order.

Chenjp avatar Apr 17 '24 02:04 Chenjp

@AlbumenJ Would you please help to review PR #14091 and share your comments.

Chenjp avatar Apr 24 '24 07:04 Chenjp