Skip to content

fix: ensure each ProcessorSlotChain gets independent slot instances#3620

Open
EvanYao826 wants to merge 1 commit into
alibaba:masterfrom
EvanYao826:fix/spi-singleton-chain-v2
Open

fix: ensure each ProcessorSlotChain gets independent slot instances#3620
EvanYao826 wants to merge 1 commit into
alibaba:masterfrom
EvanYao826:fix/spi-singleton-chain-v2

Conversation

@EvanYao826
Copy link
Copy Markdown

Fixes #3007

Problem

When building a ProcessorSlotChain, singleton slots (isSingleton=true in @Spi annotation) 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.

Reproduction: When a non-singleton ProcessorSlot (A) is placed after a singleton ProcessorSlot (B), all chains share B's next pointer, 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 independent next pointers regardless of whether the slot is a singleton.

Changes

  • DefaultSlotChainBuilder.java: Added newSlot() 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).

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
@weixq709
Copy link
Copy Markdown

slot对象只能用spi加载吧。关键在于原型对象前面如果是单例对象,不同的链修改的是同一个指针,要让每条链修改不同的指针才能解决这个问题。

@EvanYao826
Copy link
Copy Markdown
Author

Thanks for the feedback! You are right, the core issue is that different chains share the same slot instance and its next pointer.

This PR fixes it by using reflection (getDeclaredConstructor().newInstance()) to create independent slot instances for each chain, instead of directly using the SPI-loaded singleton. Each chain gets its own fresh instance, so next pointers do not interfere with each other.

SPI loading is still used as the entry point for discovering slot types (lookUpSpi), but the actual instance creation goes through reflection to avoid reusing singletons.

@weixq709
Copy link
Copy Markdown

weixq709 commented May 19, 2026

这样做的话会破坏单例slot对象的共享状态。如果期望在单例对象中缓存一些数据,但是由于每次创建新的对象,会破坏这种共享状态,这与期望不符。我尝试过使用原型对象包裹单例对象,并通过代理来解决这个问题。但是尝试后发现通过代理触发方法后,只能触发第一个节点,从而导致整调用链路断裂。因为next指针只能保存在Wrapper对象中,而不是delegate对象中。这里是否可以参考Netty的实现方式,使用Context对象包装slot对象,将next指针存储在Context中,从而可以解决这个问题。我有询问过第一种方案的意见,但是官方没有回复我。期待你的建议。

@EvanYao826
Copy link
Copy Markdown
Author

@weixq709 感谢详细的反馈!你说得很对,反射创建新实例确实会破坏单例slot的共享状态。

我分析了当前所有内置slot的实例字段:

  • NodeSelectorSlot.map — 每资源级别,chains是per-resource的,所以新实例不影响
  • ClusterBuilderSlot.clusterNode — 同样是per-resource级别
  • FlowSlot.checker — 无状态(final,构造器注入)
  • StatisticSlot — 无实例级缓存状态

当前内置slot都没有需要跨chain共享的实例状态,所以反射方案在现有代码下是安全的。

你提出的Context/Wrapper模式(参考Netty的ChannelPipeline)确实是更好的长期方案:每个slot包裹在一个携带自己next指针的node中,底层slot实例保持共享。我已经在代码注释中记录了这个方向。

对于这个PR,我建议:

  1. 先合入反射方案解决当前的bug(*bug report* ProcessorSlot的Spi中的isSingleton = false设置不生效 #3007
  2. 如果后续有slot需要跨chain共享状态,再开新PR实现Context/Wrapper模式

你觉得这个方案可以接受吗?

@weixq709
Copy link
Copy Markdown

我觉得还是需要官方认可这个方案。我刚参与开源,没有太多的经验。我这里指的是开发人员自己定义的单例slot如果需要缓存一些数据可能会破坏共享状态的问题,内置的组件不会出现这个问题。我的建议是等待官方反馈,认可这个方案后再进行修复,我们应该尽量不去引入一些新的问题。

@EvanYao826
Copy link
Copy Markdown
Author

@weixq709 采纳了你的建议,已改用Context/Wrapper模式!

实现方案(参考Netty的ChannelPipeline):

  1. ProcessorSlotContext<T> — 包装类,持有自己的next指针,底层slot保持共享单例
  2. CHAIN_NEXT_OVERRIDE ThreadLocal — 在AbstractLinkedProcessorSlot.fireEntry()/fireExit()中,当wrapper设置了ThreadLocal时,遍历重定向到wrapper的next,而非delegate自己的next
  3. 链路遍历流程:
    wrapper1.entry() → 设置CHAIN_NEXT=wrapper2 → 调用delegate.entry()
    → delegate.fireEntry() → 读取CHAIN_NEXT=wrapper2 → wrapper2.transformEntry()
    → wrapper2.entry() → 设置CHAIN_NEXT=wrapper3 → ...
    

关键优势:

  • 单例slot的共享状态完全保留(如NodeSelectorSlot.mapClusterBuilderSlot.clusterNode
  • 每个chain的wrapper有独立的next指针
  • 无反射,无新实例创建

修改文件:

  • AbstractLinkedProcessorSlot.java — 添加ThreadLocal重定向
  • ProcessorSlotContext.java — 新增wrapper类
  • DefaultSlotChainBuilder.java — 用wrapper替代反射
  • DefaultSlotChainBuilderTest.java — 验证wrapper独立性+delegate共享性

已推送更新。请review!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants