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

agent 类增强后 netty的线程池报错 #133

Closed
woshioosm opened this issue Apr 8, 2019 · 12 comments
Closed

agent 类增强后 netty的线程池报错 #133

woshioosm opened this issue Apr 8, 2019 · 12 comments

Comments

@woshioosm
Copy link

woshioosm commented Apr 8, 2019

netty的线程池 默认使用的是 MemoryAwareThreadPoolExecutor 该Executor中定义了 自己的runnable实现MemoryAwareRunnable。
使用ttl增强后报错:

java.lang.ClassCastException: com.alibaba.ttl.TtlRunnable cannot be cast to org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor$MemoryAwareRunnable
	at org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor.decreaseCounter(MemoryAwareThreadPoolExecutor.java:517)
	at org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor.beforeExecute(MemoryAwareThreadPoolExecutor.java:466)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1139)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)

具体报错就是 netty 想把task转成自己的runnable是类型错误
increment = ((MemoryAwareRunnable) task).estimatedSize;

同理 对于其他自定义线程池和runnable的程序 , 通过增强使用TtlRunnable 是否有风险?

@woshioosm woshioosm changed the title 类增强后 使得netty的线程池报错 agent 类增强后 netty的线程池报错 Apr 8, 2019
@oldratlee
Copy link
Member

oldratlee commented Apr 9, 2019

@woshioosm

这个 Issue #121 (comment) 也涉及这个问题,TTL的解决方法是:

当然背后,要做好的是:

pool的beforeExecute方法 agent也要改好,
传的runnable参数已经去TtlRunnable的wrapper了。
真正不感知TtlRunnable。

需要 可以把这个需求做一下,发个TTL的版本 :)

@oldratlee
Copy link
Member

oldratlee commented Apr 9, 2019

@woshioosm

现在 在TTL外围做的一个解决方法是:升级到 Netty 4

  • Netty 4中已经没有 org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor
  • 这个是Netty 3实现中有的类。

当然 升级Netty要做的事 也很不少,九成不是一件方便完成的事。 😄

@oldratlee oldratlee self-assigned this Apr 9, 2019
@oldratlee oldratlee added 💪 enhancement ❓question Further information is requested labels Apr 9, 2019
@woshioosm
Copy link
Author

woshioosm commented Apr 16, 2019

@oldratlee

  1. 如果是api的角度 使用ttlRunable等方式wrap感觉很合适 ,不会影响其他的executor中操作自己的Runable。
  2. 但是既然使用了字节码 能否切的更下层一些, 比如说 runable callable的方法最终都是由 futureTask的run方法中的call 来进行调用 是否可以对其进行增强(增强一处可以同时满足runnable和callable)
  3. 基于v2.10.2 jdk8 进行了实验 写了一个简答的transformor 完全仿制了ttlrunable的切面方式
    https://github.com/woshioosm/transmittable-thread-local/blob/dev/src/main/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlFutureTaskTransformlet.java
    agent只使用这一个transformletList.add(new TtlFutureTaskTransformlet()); 即可
    代码没做复杂的考虑,仅简单进行了Executor线程池的实验ok
  4. 这种方式应该对Executor 也不需要进行什么操作,也不需要创造新的类 是否可以作为一种方式进行考虑? 可以讨论下。

@oldratlee
Copy link
Member

oldratlee commented Apr 17, 2019

2. 但是既然使用了字节码 能否切的更下层一些, 比如说RunableCallable的方法最终都是由FutureTaskrun方法中的call来进行调用 是否可以对其进行增强(增强一处可以同时满足RunableCallable
3. 基于v2.10.2 jdk8 进行了实验 写了一个简答的transformor完全仿制了TtlRunable的切面方式

@woshioosm 很好的思路和实现验证! 👍 我也想一下 ❤️

这样的做法,目前想到的可能需要注意的问题:

  1. ThreadPoolExecutor的实现细节相关(不是面向API),需要跟进不同版本JDKThreadPoolExecutor实现方式调整,否则可能Break功能。
    • 简单检查了一下,JDK 6/8/11上ThreadPoolExecutor都是使用FutureTask实现的,实现变化的可能性很小。
  2. 与用户已使用的Wrap使用方式(如 TtlRunnable.get提交Runnable),如何与开启TtlAgent后的协作(如 不期望多次重复的Wrap) 要注意。

@woshioosm
Copy link
Author

woshioosm commented Apr 17, 2019

@oldratlee

  1. 嗯 是的 jdk需要 保持关注FutureTask的 result = c.call();这行不能发生变化。 个人觉得可以做些判断, 如果它不存在了 ,再生效替补的 transformor。

  2. 对于如果用户使用api的情况,可以添加判断逻辑 参见:https://github.com/woshioosm/transmittable-thread-local/blob/dev/src/main/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlFutureTaskTransformlet.java
    在futureTask的构造方法处判断是否是Ttl包裹后的runnbale和callable, 在进行后面的逻辑

String consCode2 = "isApiTtl=($1 instanceof com.alibaba.ttl.TtlCallable || $1 instanceof com.alibaba.ttl.TtlRunnable)?true:false;";
  1. 我们本身是一个追踪组件, 所以不能要求用户进行runnable api改造,也不能要求用户升级netty4 ,也不能保证用户没有和netty3一样 自己对线程池进行了处理, 比较头疼。

@auyang-0626
Copy link

auyang-0626 commented Jun 3, 2019

@oldratlee 你好,

我们在trace系统中引入了ttl.同样遇到用户在beforeExecutor方法中,强转runnable的类型报错。

我的解决方法是

agent增强的时候,拦截所有的ExecutorService派生出来的类,把它们的beforeExecutor和afterExecutor都增强下,从TtlRunnable取出用户原始的类型,然后还回去。

这种方式你觉得会有什么隐患吗?

@oldratlee
Copy link
Member

oldratlee commented Jul 11, 2019

@woshioosm @auyang-0626

在版本 2.11.0 中,已经解决了这个问题。
https://github.com/alibaba/transmittable-thread-local/releases/tag/v2.11.0


实现方式 即 是你的思路: @auyang-0626

agent增强的时候,拦截所有的ExecutorService派生出来的类,把它们的beforeExecutor和afterExecutor都增强下,从TtlRunnable取出用户原始的类型,然后还回去。

另外加的判断逻辑是:

只在Agent的情况下(即Auto Wrapper),才会做对应的这个Auto Unwrap操作。

@DreamLettuce
Copy link

DreamLettuce commented Mar 16, 2022

@woshioosm @auyang-0626

在版本 2.11.0 中,已经解决了这个问题。 https://github.com/alibaba/transmittable-thread-local/releases/tag/v2.11.0

实现方式 即 是你的思路: @auyang-0626

agent增强的时候,拦截所有的ExecutorService派生出来的类,把它们的beforeExecutor和afterExecutor都增强下,从TtlRunnable取出用户原始的类型,然后还回去。

另外加的判断逻辑是:

只在Agent的情况下(即Auto Wrapper),才会做对应的这个Auto Unwrap操作。

String code = String.format(
// decorate to TTL wrapper,
// and then set AutoWrapper attachment/Tag
" $%d = %s.get($%1$d, false, true);"
+ "\n com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.setAutoWrapperAttachment($%1$d);",
i + 1, PARAM_TYPE_NAME_TO_DECORATE_METHOD_CLASS.get(paramTypeName));
insertCode.append(code);

看代码是根据TtlAttachments.KEY_IS_AUTO_WRAPPER 标记判断是否为Auto Wrapper
但是上方增强代码是直接调用的setAutoWrapperAttachment方法,
并未判断是否被agent真正的增强成功。

如果用户调用线程池execute/submit时传参类型为TtlRunnable,则实际未被agent增强
(因为TtlRunnable.get中有重复增强判断),
却在beforeExecute/afterExecute中执行了unwrap逻辑
(因为TtlRunnable本身实现了TtlAttachments)。

这样的话,如果用户在自定义beforeExecute/afterExecute中做了类似的强转动作,
还是会报错ClassCastException

不知道理解的对不对,求大佬解答。 @oldratlee


测试code:

public static void main(String[] args) {
    MyThreadPoolExecutor myThreadPoolExecutor = new MyThreadPoolExecutor(1, 1, 1, TimeUnit.MINUTES, new LinkedBlockingQueue<>());
    myThreadPoolExecutor.execute(TtlRunnable.get(() -> System.out.println("agent test")));
}


static class MyThreadPoolExecutor extends ThreadPoolExecutor {

    public MyThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue) {
        super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue);
    }

    @Override
    protected void beforeExecute(Thread t, Runnable r) {
        TtlRunnable ttlRunnable = (TtlRunnable) r;
        super.beforeExecute(t, r);
    }
}

@oldratlee
Copy link
Member

oldratlee commented Mar 19, 2022

@DreamLettuce 问题收到!

不知道理解的对不对,求解答

你的理解及其分析 是对的,非常专业! 👍 ❤️ @DreamLettuce

这是 一个TTLBug

TTL Agent使用方式下,对于用户显式传入的TtlRunnable任务实例:

  • beforeExecute/afterExecute中应该保留类型
  • TTL Agent不应该做unwrap

我尽快 release 一个版本,修复问题。


对于独立的问题(如实现Bug),可以开个新的 issue。 🙏 @DreamLettuce

也可以方便更多人看到。

PS 关于,是否有实际的业务使用场景?

beforeExecute/afterExecute中做强转TtlRunnable,即TtlRunnable ttlRunnable = (TtlRunnable) r;
这样做的 实际业务需求场景是什么? ❤️ @DreamLettuce

应该有 更简单安全、更业务显式语义 的做法。


可能因为这样的使用方式不常见
(在beforeExecute/afterExecute中做强转TtlRunnable),
所以 这个Bug没有在业务使用中有效地暴露出来 😖

@oldratlee
Copy link
Member

oldratlee commented Mar 19, 2022

@DreamLettuce

问题复现

在复现提交 32adad1 的单元测试中,添加了复现的Test Case

/**
* for bug submitted by
* https://github.com/alibaba/transmittable-thread-local/issues/133#issuecomment-1068793261
*/
@Test
@ConditionalIgnore(condition = NoAgentRun::class)
fun underAgent_task_is_explicit_TtlRunnable__should_not_be_unwrapped() {
val myThreadPoolExecutor = MyThreadPoolExecutor()
(0 until 10).map {
val r = TtlRunnable.get(MyRunnable())!!
myThreadPoolExecutor.execute(r)
}
Thread.sleep(100)
assertEquals(20, myThreadPoolExecutor.runnableList.size)
assertTrue(myThreadPoolExecutor.runnableList.all { it is TtlRunnable })
}

集成测试失败并复现问题:

There was 1 failure:
1) underAgent_task_is_explicit_TtlRunnable__should_not_be_unwrapped(com.alibaba.test.ttl.threadpool.BeforeAndAfterExecuteMethodOfExecutorSubclassTest)
java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at com.alibaba.test.ttl.threadpool.BeforeAndAfterExecuteMethodOfExecutorSubclassTest.underAgent_task_is_explicit_TtlRunnable__should_not_be_unwrapped(BeforeAndAfterExecuteMethodOfExecutorSubclassTest.kt:75)
FAILURES!!!

https://ci.appveyor.com/project/oldratlee/transmittable-thread-local/builds/42950727/job/cwwfxds3f0mx0d93#L555

问题修复

发布

发了v2.12.6
https://github.com/alibaba/transmittable-thread-local/releases/tag/v2.12.6

<dependency>
    <groupId>com.alibaba</groupId>
    <artifactId>transmittable-thread-local</artifactId>
    <version>2.12.6</version>
</dependency>

@DreamLettuce 可以用一下这个新版本,确认是否解决你的问题。

@DreamLettuce
Copy link

DreamLettuce commented Mar 21, 2022

PS 关于,是否有实际的业务使用场景?

beforeExecute/afterExecute中做强转TtlRunnable,即TtlRunnable ttlRunnable = (TtlRunnable) r;, 这样做的 实际业务需求场景是什么? ❤️ @DreamLettuce

目前没有实际的业务场景,只是我们目前使用TTL(v2.10.x)时,遇到当前issue netty线程池强转的bug,查看社区fix代码时发现的这个小问题。

我也觉得一般情况下业务使用了agent不会再直接调用TtlRunnable对象,但是也有可能先使用了Java SDK,后续如果改为使用agent可能会暴露这个问题。

@oldratlee
Copy link
Member

oldratlee commented Mar 21, 2022

目前没有实际的业务场景,只是我们目前使用TTL(v2.10.x)时,遇到当前issue netty线程池强转的bug,查看社区fix代码时发现的这个小问题。

推荐跟进升级,避免再踩新版本中已经解决了的问题。@DreamLettuce

具体看各个版本 release note 中的说明:
https://github.com/alibaba/transmittable-thread-local/releases

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

4 participants