Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable Inheritable when it's not necessary and buggy(eg. has potential memory leaking problem) #100

Closed
oldratlee opened this issue Jul 31, 2018 · 5 comments
Assignees

Comments

@oldratlee
Copy link
Member

oldratlee commented Jul 31, 2018

The core scenario is transmit value value from the time task is created to the time task is executed, so transmitting value new Thread(Inheritable) maybe is not necessary and buggy(eg. has potential memory/context leaking problem).

But when use thread pool, thread is cached up and used repeatedly. Transmitting value from parent thread to child thread has no meaning. Application need transmit value from the time task is created to the time task is executed.

@driventokill
Copy link
Member

Transmit thread local values by inheritable thread local is a potential leaking problem. A new created thread can never know how to deal with the inherited values. The transmittable thread local values should be created and cleared within a task's lifecycle. InheritableThreadLocal will leak the management of transmittable thread local values out of a task.

driventokill added a commit that referenced this issue Oct 8, 2018
Inheritable feature by using InheritableThreadLocal can be disabled by
setting `com.alibaba.ttl.inheritable` to false to avoid potential
leaking problem described in #100, default value is true to ensure
compatible with previous versions.
driventokill added a commit that referenced this issue Oct 11, 2018
    Inheritable feature by using InheritableThreadLocal can be disabled by
    overriding `childValue` and `return null` to avoid potential
    leaking problem described in #100. Implementation is provided as
    NoInheritableTTL.
@driventokill
Copy link
Member

driventokill commented Oct 12, 2018

Here are some thoughts about the inheritable feature:

case 1

Inheritable feature in thread pooling components(ThreadPoolExecutor etc.) should never happen, because threads in thread pooling components is pre-created and pooled, so these threads is neutral for biz logic/data.

case 2

Whether the value should be inheritable or not can be controlled by the data owner, disable it carefully when data owner have a clear idea.

disable inheritable by overriding the childValue and return initialValue():

TransmittableThreadLocal<String> transmittableThreadLocal = new TransmittableThreadLocal<String>() {
     protected String childValue(String parentValue) {
         return initialValue();
     }
 }

oldratlee added a commit that referenced this issue Nov 2, 2018
- add DisableInheritableThreadFactory wrapper in TtlExecutors
- add Inheritable unit test
oldratlee pushed a commit that referenced this issue Nov 2, 2018
…Local

disable Inheritable feature by using InheritableThreadLocal
can avoid potential leaking problem described in #100
@oldratlee oldratlee changed the title add option for TTL, disable Inheritable disable Inheritable when it's not necessary and buggy(eg. has potential memory leaking problem) Nov 2, 2018
@driventokill
Copy link
Member

Solved.

@youzipi
Copy link

youzipi commented Jan 9, 2019

为什么不直接 继承 ThreadLocal 呢?
我理解的 TTL 和 ITL 是解决不同场景的方案,从出发点就不一样。

或者可以在 构造器 增加参数-inheritable,默认为 false

@oldratlee
Copy link
Member Author

oldratlee commented Jan 9, 2019

@youzipi

为什么不直接 继承 ThreadLocal 呢?

在业务中可以直接以new Thread这样的方式 来 使用,正确完成传递 是 期望的功能。
# 虽然,有了线程池,直接new线程的做法,极有可能是不应该的(不是最佳实践!)。

选择继承ITL而不直接继承ThreadLocal,原因如下:

  • new Thread的情况下,也能进行传递。即包含上面new Thread的传递功能。
  • 继承ThreadLocal,对于使用了ITL的业务代码,则不方便切成TTL了。

inheritable 在业务上 有潜在的(potential)的 泄漏或污染。
这样的情况下,期望进一步方便地解决好~

我理解的 TTL 和 ITL 是解决不同场景的方案,从出发点就不一样。

要说明/解决问题,你需要给出 全场景 下的说明,比如说明出:
TTL/ITL在继承上下文有泄漏或污染问题时,应该全部使用成ThreadLocal对应的业务功能 也都能解决好。

那么,就可以推出你的结论:『 TTLITL是解决不同场景的方案,从出发点就不一样』。

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

No branches or pull requests

3 participants