Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,39 @@ public ProcessorSlotChain build() {
continue;
}

chain.addLast((AbstractLinkedProcessorSlot<?>) slot);
// Always create a new instance of the slot to ensure each chain has its own
// copy with independent next pointers. This is necessary because singleton slots
// (isSingleton=true in @Spi annotation) are shared across all build() calls.
// If a singleton slot is directly used in multiple chains, modifying its 'next'
// pointer when building one chain will affect all other chains that use it.
// See: https://github.com/alibaba/Sentinel/issues/3007
AbstractLinkedProcessorSlot<?> newSlot = newSlot(slot);
chain.addLast(newSlot);
}

return chain;
}

/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a fresh instance of every slot per chain ignores the @SPI(isSingleton=true) contract. Built-in singleton slots (StatisticSlot, FlowSlot, DegradeSlot, SystemSlot, AuthoritySlot, LogSlot, DefaultCircuitBreakerSlot) hold no required shared instance state, but custom singleton slots that rely on shared instance-level state will be silently copied per chain, breaking their shared-state semantics.

* Create a new instance of the given slot.
* <p>
* This method creates a fresh instance of the slot's concrete class via reflection,
* ensuring that each chain gets its own independent copy of every slot. This prevents
* the issue where singleton slots shared across multiple chains would have their
* {@code next} pointer overwritten by the last chain built.
* </p>
*
* @param slot the slot to create a new instance of
* @return a new instance of the slot, or the original slot if creation fails
*/
@SuppressWarnings("unchecked")

@oss-sentinel-ai oss-sentinel-ai Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Info] Retracted: my earlier comment on this line was incorrect — the {@code next} Javadoc reference here is well-formed. No action needed. Apologies for the noise.


Automated correction by github-manager-bot

private static AbstractLinkedProcessorSlot<?> newSlot(ProcessorSlot slot) {
try {
return slot.getClass().getDeclaredConstructor().newInstance();

@oss-sentinel-ai oss-sentinel-ai Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Info] Retracted: my earlier comment was incorrect — the @param slot tag is well-formed. No action needed. Apologies for the noise.


Automated correction by github-manager-bot

} catch (Exception e) {
RecordLog.warn("Failed to create new instance of ProcessorSlot("
+ slot.getClass().getCanonicalName() + "), using original instance", e);
return (AbstractLinkedProcessorSlot<?>) slot;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reflection (getDeclaredConstructor().newInstance()) for every slot on every build() adds overhead and will fail for slots whose no-arg constructor is not accessible. Although build() is invoked once per resource, this approach also bypasses any initialization or lifecycle guarantees provided by SpiLoader.

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block returns the original SPI singleton instance when reflective construction fails. This silently reintroduces issue #3007 for that slot because its 'next' pointer is still shared across all ProcessorSlotChain instances, so one chain build will corrupt the topology of previously built chains.

Loading