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

如果 用户重写阻塞队列 offer方法,这种情况能够兼容么 #340

Closed
simake2017 opened this issue Dec 28, 2021 · 4 comments
Closed
Assignees
Labels
📐 design discussion 😖 no runnable reproducible demo 😵 please provide a simple runnable demo that reproduce the problem ❓question Further information is requested

Comments

@simake2017
Copy link

image

这里用户重写了一个阻塞队列,然后重写了put和offer方法,里面进行了强制转换,这种现在能兼容么

@oldratlee oldratlee added the ❓question Further information is requested label Dec 28, 2021
@oldratlee
Copy link
Member

oldratlee commented Dec 28, 2021

@simake2017 给一个可以说明/复现问题的、极简可运行的Demo工程:❤️


PriorityBlockingQueue的问题

@simake2017
Copy link
Author

simake2017 commented Dec 28, 2021

是这样的:

  • 用户自己写了一个阻塞队列,然后在创建ThreadPoolExecutor的时候,使用了自己的阻塞队列
  • 阻塞队列中,又重写了put以及offer方法,方法内容如上图,里面会进行强转
    • 也就是将TtlRunnable转为自己的Runnable子类
    • 就会爆出ClassCastException

可运行demo,稍后给出,问题应该比较清楚的

@oldratlee
Copy link
Member

oldratlee commented Dec 28, 2021

编码的解决方法,已经有支持的:

Runnable参数 先做TtlRunnable.unwrap(runnable)

@simake2017 应该是你说的兼容,兼容这样的使用方式?

演示Demo

写了一个说明问题的、极简可运行的Demo,在分支dev-2.12.x上: @simake2017

@Override
public boolean offer(final Runnable runnable) {
// unwrap TtlRunnable first
final Runnable unwrap = TtlRunnable.unwrap(runnable);
final MyTask myTask = (MyTask) unwrap;
if (myTask.msg.startsWith("accept-")) {
// ignore result/return value of offer, BAD!!
// does not follow the contract of method offer
// this is just a simple demo!
super.offer(runnable);
}
// always return true even if discard the task, BAD!!
// does not follow the contract of method offer
// this is just a simple demo!
return true;
}
@Override
public void put(final Runnable runnable) throws InterruptedException {
// unwrap TtlRunnable first
final Runnable unwrap = TtlRunnable.unwrap(runnable);
final MyTask myTask = (MyTask) unwrap;
// discard task if not satisfied, BAD!!
// does not follow the contract of method put
// this is just a simple demo!
if (myTask.msg.startsWith("accept-")) {
super.put(runnable);
}
}

关于系统设计的讨论

就像代码注释中所说的,

final MyTask myTask = (MyTask) unwrap;
if (myTask.msg.startsWith("accept-")) {
// ignore result/return value of offer, BAD!!
// does not follow the contract of method offer
// this is just a simple demo!
super.offer(runnable);
}
// always return true even if discard the task, BAD!!
// does not follow the contract of method offer
// this is just a simple demo!
return true;

  • 丢弃任务、忽略真实的返回结果(任务是否提交成功了)的做法是有问题的:
    不符合BlockingQueue接口方法的契约。
  • 结果可能会带来各种细节、下层基础的问题(挖坑了)! 💣 💥

这可能是 平时我们几乎没有看到定制/继承BlockingQueue 的原因。 @simake2017

看起来,像任务过滤这样的业务逻辑:

  • 应该在上层的线程池中(比如wrapperAOP线程池)完成,
  • 而不是定制下层的BlockingQueue但打破了BlockingQueue接口方法契约的方式来实现。

@oldratlee oldratlee self-assigned this Dec 28, 2021
@oldratlee oldratlee added the 😖 no runnable reproducible demo 😵 please provide a simple runnable demo that reproduce the problem label Dec 30, 2021
huangfei1101 pushed a commit to huangfei1101/transmittable-thread-local that referenced this issue Dec 31, 2021
huangfei1101 pushed a commit to huangfei1101/transmittable-thread-local that referenced this issue Dec 31, 2021
huangfei1101 pushed a commit to huangfei1101/transmittable-thread-local that referenced this issue Dec 31, 2021
author Jerry Lee <[email protected]> 1640756168 +0800
committer 飞竑 <[email protected]> 1640940421 +0800

parent b6b98bf
author Jerry Lee <[email protected]> 1640756168 +0800
committer 飞竑 <[email protected]> 1640940365 +0800

parent b6b98bf
author Jerry Lee <[email protected]> 1640756168 +0800
committer 飞竑 <[email protected]> 1640940132 +0800

parent b6b98bf
author Jerry Lee <[email protected]> 1640756168 +0800
committer 飞竑 <[email protected]> 1640939931 +0800

refactor: deprecated methods invoke new methods, not vice versa

提交TtlWrappers的说明demo;

test: add demo code `CustomizedBlockingQueueWithTtlDemo` for alibaba#340

tests: add unit test for new methods `TtlWrappers.wrap*`

refactor: deprecated methods invoke new methods, not vice versa

test: add demo code `CustomizedBlockingQueueWithTtlDemo` for alibaba#340

tests: add unit test for new methods `TtlWrappers.wrap*`

refactor: deprecated methods invoke new methods, not vice versa
@oldratlee
Copy link
Member

@simake2017 这个 Issue 先 close。如果还有问题,可以继续讨论。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📐 design discussion 😖 no runnable reproducible demo 😵 please provide a simple runnable demo that reproduce the problem ❓question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants