Skip to content

fix: isolate ProcessorSlotChain links from shared SPI slot instances - #3646

Open
TwistedRiCen wants to merge 1 commit into
alibaba:1.8from
TwistedRiCen:issue-3007-slot-context
Open

fix: isolate ProcessorSlotChain links from shared SPI slot instances#3646
TwistedRiCen wants to merge 1 commit into
alibaba:1.8from
TwistedRiCen:issue-3007-slot-context

Conversation

@TwistedRiCen

Copy link
Copy Markdown

What

Fixes #3007.

Every slot inserted into a DefaultProcessorSlotChain is now added through a chain-local ProcessorSlotContext wrapper that owns the per-chain next reference. The SPI-managed slot instance — singleton or prototype — is never linked into chain topology, so building one chain can no longer corrupt another.

Problem

Slots annotated @Spi(isSingleton = true) are loaded once and shared by every chain built from DefaultSlotChainBuilder. addLast/addFirst link nodes by writing the slot's next field:

end.setNext(protocolProcessor); // writes the shared singleton's next

When a shared singleton slot is followed by a per-chain (prototype) slot, building a second chain overwrites the singleton's next, so the first chain silently executes the second chain's successor:

  • chain1: B → A1
  • chain2: B → A2 (sets B.next = A2)
  • chain1 now executes [B, A2]A1 is lost.

This affects any deployment that registers custom slots via SPI where a singleton slot precedes a prototype slot (repro in #3007; also confirmed on current 1.8 HEAD).

Root cause

Chain topology (the next pointers) is stored on the slot instances themselves, but an instance's sharing scope (SPI singleton vs. prototype) is independent of a chain's scope. A shared singleton followed by a per-chain successor means pointer aliasing across chains.

Design

  1. Chain-local wrappers own the topology. DefaultProcessorSlotChain#addFirst/addLast wrap each slot in a ProcessorSlotContext (pass-through if it is already one) and link the wrappers — never the delegates. The SPI-loaded instance keeps its singleton semantics: it is shared, but it is no longer part of any chain's link structure.

  2. Scoped successor context for delegated propagation. Around each delegation, the wrapper activates a ThreadLocal ChainContext (source = delegate, next = the wrapper's successor), restored in finally. AbstractLinkedProcessorSlot#getNext() resolves through the active context when it is the source, and falls back to the instance field otherwise. This makes the inherited fireEntry()/fireExit() — and any custom override that propagates via getNext() — continue down the calling chain instead of the delegate's stale field.

Why not the alternatives

  • Re-instantiate singleton slots per chain via reflection (fix: ensure each ProcessorSlotChain gets independent slot instances #3620): breaks the isSingleton = true contract for custom slots that keep instance-level state; fails for non-public classes; bypasses SpiLoader's lifecycle; and any fallback to the shared instance re-introduces exactly this bug. See the review discussion there.
  • Wrapper only (Fix invalid prototype slot node issue #3616): fixes the write side, but a delegate calling the inherited fireEntry() propagates through its own next field, so traversal stops at the first wrapped node unless fire-entry propagation is bridged — which is what the scoped successor context in this PR adds.

Behavior preserved

  • Singleton delegates are still shared across chains (asserted by tests).
  • Prototype slots are still instantiated per chain.
  • Code that links slots manually outside DefaultProcessorSlotChain (direct setNext) behaves exactly as before: no context is active, so getNext() returns the instance field.

Trade-offs and limitations

  • Wrapper visibility during traversal: slotChain.getNext() returns ProcessorSlotContext, so instanceof-based node inspection must unwrap via getDelegate() (DefaultSlotChainBuilderTest updated accordingly).
  • Per-invocation cost: one ThreadLocal push/pop plus one small immutable context allocation per slot entry/exit. Measurable only on very hot paths; can be pooled or flattened if maintainers prefer.
  • Synchronous propagation only: the bridge lives for the duration of the delegation (push/pop in finally). A slot that fires fireEntry()/fireExit() asynchronously — e.g. on another thread, or after the delegation returns — would not see the context. Core slots propagate synchronously, but this is a semantic edge worth documenting.

Testing

  • sentinel-core: full suite green — 228 tests, 0 failures, 0 errors, 0 skipped.
  • Full reactor mvn test (re-verified 2026-09-04 on current 1.8 HEAD): all shipped modules green — core, all extension/datasource modules, all adapters, transport, cluster, dashboard, logging, and demos. Two tests excluded as environmentally unavailable locally and unrelated to this change: ConsulDataSourceTest (requires Docker via Testcontainers) and NativeGreetingResourceIT (requires a pre-built GraalVM native image).
  • New DefaultProcessorSlotChainTest cases:
    • the *bug report* ProcessorSlot的Spi中的isSingleton = false设置不生效 #3007 repro: shared singleton B with per-chain successors; entry and exit order asserted; wrappers are distinct, delegate is the same shared instance;
    • nested chains invoked from within a slot;
    • repeated fireEntry within one slot;
    • context restored after an exception (no ThreadLocal leak; manual linking still works afterwards);
    • 32 concurrent chains × 100 invocations on 8 threads, with per-chain counters and leak assertions.
  • DefaultSlotChainBuilderTest extended: built-in slot order preserved through wrappers; prototype delegates differ per chain, singleton delegates are identical.

Relationship to other PRs

@CLAassistant

CLAassistant commented Sep 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@TwistedRiCen
TwistedRiCen force-pushed the issue-3007-slot-context branch from 7a3d4ae to 28fbd0b Compare September 4, 2026 14:52
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.

*bug report* ProcessorSlot的Spi中的isSingleton = false设置不生效

2 participants