refactor: plugin service lifecycle safety — @ServiceAvailability annotation and hollow-service init guards#10228
Open
sridhar-panigrahi wants to merge 2 commits intobesu-eth:mainfrom
Conversation
Contributor
Author
|
@fab-10 , please let me know your thoughts on this ! |
…ceAvailability annotation Addresses a core pain point in the Besu Plugin API where a "hollow service" pattern caused silent NullPointerExceptions: BlockchainService, TransactionSimulationService, and RpcEndpointService are registered early (REGISTERING phase) but fully initialized only in the STARTED phase after BesuController is built. Plugins calling query methods before start() received NullPointerExceptions with no guidance. Changes: plugin-api: add ServiceLifecyclePhase enum documenting the ordered phases of the plugin lifecycle with Javadoc on which services are available at each phase. plugin-api: add @ServiceAvailability annotation (RUNTIME retention) that service interfaces use to declare availableFrom and fullyInitializedFrom phases. Applied to BlockchainService, TransactionSimulationService, RpcEndpointService (availableFrom=REGISTERING, fullyInitializedFrom=STARTED) and BesuEvents, BesuConfiguration, PicoCLIOptions, StorageService, SecurityModuleService, PermissioningService, MetricCategoryRegistry (availableFrom=REGISTERING). BlockchainServiceImpl: add checkInitialized() guard on all query methods. Before init() is called, these throw IllegalStateException with a clear message pointing to the STARTED phase rather than a confusing NullPointerException. TransactionSimulationServiceImpl: same pattern applied to simulatePendingBlockHeader() and simulate() overloads. BesuPluginContextImpl: - state field changed to volatile for thread-visible lifecycle transitions - serviceRegistry switched from HashMap to ConcurrentHashMap for safe concurrent access - addService() emits WARN when a service is overwritten with a different instance - getService() emits DEBUG with current lifecycle phase when a service is not found Tests: BlockchainServiceImplTest, TransactionSimulationServiceImplTest, BesuPluginContextImplTest covering init guards, overwrite detection, missing service returns, and concurrent access safety. Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>
1ba1caf to
58cb9f6
Compare
…ycle safety at runtime The @ServiceAvailability annotation introduced previously was purely declarative — it documented when a service becomes fully usable but nothing in the runtime path read it. This meant a plugin could call blockchain.getChainHeadHash() during register() and still receive a NullPointerException with no guidance. This commit wires the annotation into getService() so it actively protects plugin developers: BesuPluginContextImpl.getService() now reads @ServiceAvailability at runtime via reflection. If a plugin requests a service that is registered but not yet fully initialized (fullyInitializedFrom phase not yet reached), it emits a WARN log: "Plugin is accessing BlockchainService during lifecycle phase 'REGISTERING', but this service is not fully initialized until the 'STARTED' phase. Store the service reference in register() and defer method calls to start()." The service is still returned (not suppressed) so existing plugins are not broken, but the warning gives developers immediate, actionable feedback at the right moment. Two private helpers added to BesuPluginContextImpl: - isAtOrAfter(Lifecycle, ServiceLifecyclePhase): compares internal state against the public phase declared in the annotation - toPublicPhaseOrdinal(Lifecycle): maps the internal Lifecycle enum to the public ServiceLifecyclePhase ordinal for ordered comparison Tests added to BesuPluginContextImplTest: - getService_startedOnlyService_isStillReturnedDuringRegisteringPhase: verifies the service is still returned (advisory-only behavior, never suppresses) - getService_earlyService_isReturnedWithoutIssue: REGISTERING-phase services work cleanly without triggering the check - serviceAvailabilityAnnotation_isReadableAtRuntime: confirms RUNTIME retention holds and annotation fields are correct via reflection - earlyServiceAvailabilityAnnotation_hasNoFullyInitializedRestriction: confirms default fullyInitializedFrom = UNINITIALIZED disables the check Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses one of the core structural problems in the Besu Plugin API described in LFDT mentorship project #82: the plugin lifecycle during startup is not well-defined, causing plugins to access services before they are fully initialized.
Specifically, three services (
BlockchainService,TransactionSimulationService,RpcEndpointService) follow a "hollow service" pattern — they are registered in theServiceManagerduring theREGISTERINGphase so plugins can obtain a reference, but their internal state (blockchain,protocolSchedule,transactionSimulator) is only set viainit()calls instartPlugins(). Any plugin that calls query methods duringregister()receives aNullPointerExceptionwith no indication of what went wrong or how to fix it.Changes
plugin-api:ServiceLifecyclePhaseenumIntroduces a public enum documenting the ordered phases of the plugin lifecycle (
UNINITIALIZED → REGISTERING → REGISTERED → BEFORE_EXTERNAL_SERVICES → STARTED → STOPPING → STOPPED) with Javadoc listing which services are available at each phase. This is the foundation for lifecycle-aware tooling and documentation.plugin-api:@ServiceAvailabilityannotationA new
@Retention(RUNTIME)annotation for service interfaces. Declares:availableFrom— earliest phase the service is registered and accessiblefullyInitializedFrom— phase from which all methods are safe to call (defaults toavailableFrom)The
RUNTIMEretention is intentional: it allows future runtime checks, logging, and IDE plugin tooling to detect premature service access programmatically.Applied to:
availableFromfullyInitializedFromBlockchainServiceREGISTERINGSTARTEDTransactionSimulationServiceREGISTERINGSTARTEDRpcEndpointServiceREGISTERINGSTARTEDBesuEventsSTARTEDSTARTEDBesuConfiguration,PicoCLIOptions,StorageService,SecurityModuleService,PermissioningService,MetricCategoryRegistryREGISTERINGREGISTERINGBlockchainServiceImpl:checkInitialized()guardAll query methods (
getBlockByNumber,getBlockByHash,getBlockHeaderByHash,getChainHeadHash,getChainHeadHeader) now callcheckInitialized()before delegating to the blockchain. If called beforeinit(), they throwIllegalStateExceptionwith an actionable message:Instead of
NullPointerException: Cannot invoke "MutableBlockchain.getBlockByNumber(long)".TransactionSimulationServiceImpl: same guard patternsimulatePendingBlockHeader()andsimulate()overloads protected withcheckInitialized().BesuPluginContextImpl: three safety improvementsstatefield madevolatile— the lifecycle phase is now part of the public API surface (viagetLifecyclePhase()). Plugin background threads that call this need a happens-before guarantee.serviceRegistryswitched fromHashMaptoConcurrentHashMap— consistent with the recent thread-safety fix on the same class; concurrent reads during plugin startup are now safe.addService()now emits aWARNlog when a service is overwritten with a different instance — surfaces accidental overwrites that would otherwise be completely silent. Same-instance re-registration does not warn.getService()now emits aDEBUGlog including the current lifecycle phase when a service is not found — gives plugin developers the exact context they need to diagnose ordering issues.Tests
BlockchainServiceImplTest— verifiesIllegalStateExceptionon all guarded query methods beforeinit(), and that they delegate correctly afterinit().TransactionSimulationServiceImplTest— same for simulation methods.BesuPluginContextImplTest— covers first registration, same-instance re-registration (no warning), different-instance overwrite (new instance returned), missing service returnsOptional.empty(), concurrent read/write safety with 20 threads, and type validation.Relation to mentorship project #82
This PR directly targets Deliverable 2: Lifecycle Refinement — eliminating race conditions where plugins attempt to access a service before it is fully initialized. It also lays groundwork for Deliverable 3: API Versioning System through the
@ServiceAvailabilityannotation, which provides a structured, machine-readable way to express service availability contracts.