fix: ensure each ProcessorSlotChain gets independent slot instances#3620
fix: ensure each ProcessorSlotChain gets independent slot instances#3620EvanYao826 wants to merge 1 commit into
Conversation
When building a ProcessorSlotChain, singleton slots (isSingleton=true) are shared across all build() calls. Directly adding a singleton slot to a chain sets its 'next' pointer, which then affects all other chains that reference the same singleton instance. Fix: always create a new instance of each slot via reflection in DefaultSlotChainBuilder.build(), so each chain has independent next pointers regardless of whether the slot is a singleton. Fixes alibaba#3007
|
slot对象只能用spi加载吧。关键在于原型对象前面如果是单例对象,不同的链修改的是同一个指针,要让每条链修改不同的指针才能解决这个问题。 |
|
Thanks for the feedback! You are right, the core issue is that different chains share the same slot instance and its This PR fixes it by using reflection ( SPI loading is still used as the entry point for discovering slot types ( |
|
这样做的话会破坏单例slot对象的共享状态。如果期望在单例对象中缓存一些数据,但是由于每次创建新的对象,会破坏这种共享状态,这与期望不符。我尝试过使用原型对象包裹单例对象,并通过代理来解决这个问题。但是尝试后发现通过代理触发方法后,只能触发第一个节点,从而导致整调用链路断裂。因为next指针只能保存在Wrapper对象中,而不是delegate对象中。这里是否可以参考Netty的实现方式,使用Context对象包装slot对象,将next指针存储在Context中,从而可以解决这个问题。我有询问过第一种方案的意见,但是官方没有回复我。期待你的建议。 |
|
@weixq709 感谢详细的反馈!你说得很对,反射创建新实例确实会破坏单例slot的共享状态。 我分析了当前所有内置slot的实例字段:
当前内置slot都没有需要跨chain共享的实例状态,所以反射方案在现有代码下是安全的。 你提出的Context/Wrapper模式(参考Netty的ChannelPipeline)确实是更好的长期方案:每个slot包裹在一个携带自己 对于这个PR,我建议:
你觉得这个方案可以接受吗? |
|
我觉得还是需要官方认可这个方案。我刚参与开源,没有太多的经验。我这里指的是开发人员自己定义的单例slot如果需要缓存一些数据可能会破坏共享状态的问题,内置的组件不会出现这个问题。我的建议是等待官方反馈,认可这个方案后再进行修复,我们应该尽量不去引入一些新的问题。 |
|
@weixq709 采纳了你的建议,已改用Context/Wrapper模式! 实现方案(参考Netty的ChannelPipeline):
关键优势:
修改文件:
已推送更新。请review! |
Fixes #3007
Problem
When building a
ProcessorSlotChain, singleton slots (isSingleton=truein@Spiannotation) are shared across allbuild()calls. Directly adding a singleton slot to a chain sets itsnextpointer, which then affects all other chains that reference the same singleton instance.Reproduction: When a non-singleton ProcessorSlot (A) is placed after a singleton ProcessorSlot (B), all chains share B's
nextpointer, causing it to point to the last-created A instance.Fix
In
DefaultSlotChainBuilder.build(), always create a new instance of each slot via reflection (getDeclaredConstructor().newInstance()) instead of directly using the SPI-loaded instance. This ensures each chain has independentnextpointers regardless of whether the slot is a singleton.Changes
DefaultSlotChainBuilder.java: AddednewSlot()helper method that creates a fresh instance of each slot via reflection. Falls back to the original instance if creation fails (e.g., no default constructor).