Open
Conversation
jungm
reviewed
Apr 12, 2026
| // Per spec, the executor attribute may reference either a ManagedScheduledExecutorService | ||
| // or a plain ManagedExecutorService. When a plain MES is referenced, fall back to the | ||
| // default MSES for scheduling capability but preserve the MES's context service. | ||
| final ManagedScheduledExecutorServiceImpl mses = resolveMses(asynchronous.executor()); |
Member
There was a problem hiding this comment.
interestingly enough concurro completely disregargs the configured executor, claiming it is not to be used in the scheduled case according to the spec (though i cannot find where this behavior is described)
https://github.com/eclipse-ee4j/glassfish-concurro/blob/3.1.0/src/main/java/org/glassfish/concurro/cdi/asynchronous/AsynchronousInterceptor.java#L127-L132
openliberty seems to at least honor the contextservice that is attached to the used executor
| // Extract method and target from InvocationContext for direct invocation. | ||
| // We must NOT reuse ctx.proceed() — InvocationContext's interceptor iterator | ||
| // is single-use, so subsequent proceed() calls would bypass TX/security interceptors. | ||
| final Method beanMethod = ctx.getMethod(); |
Member
There was a problem hiding this comment.
this is likely not fixing what it is trying to fix as this returns the raw method and not the method on the CDI proxy that would run interceptors, pushed a test to prove this regression and working on a fix
… 3.1 Add support for scheduled recurring async methods as defined in Jakarta Concurrency 3.1. This addresses the largest block of TCK failures (28 tests) by enabling cron-based scheduling of CDI @asynchronous methods via the new runAt attribute. - ScheduleHelper: maps @schedule annotations to API-provided CronTrigger, supports composite triggers (multiple schedules) and skipIfLateBy wrapping - AsynchronousInterceptor: branches on runAt presence — one-shot path unchanged, new scheduled path uses ManagedScheduledExecutorService - ManagedScheduledExecutorServiceImplFactory: adds lookup() with graceful fallback matching the ManagedExecutorServiceImplFactory pattern
Implement opt-in virtual thread support (Java 21+) across ManagedThreadFactory, ManagedExecutorService, and ManagedScheduledExecutorService as required by Jakarta Concurrency 3.1. Runtime: - VirtualThreadHelper: reflection-based Java 21 API access, graceful fallback on Java 17 (isSupported check, no multi-release JARs) - ManagedThreadFactoryImpl: virtual path creates threads that do NOT implement ManageableThread (spec 3.4.4) - All three factory classes accept virtual property Deployment descriptor + annotation processing: - Boolean virtual field added to DD model classes in openejb-jee - Convert*Definitions pass virtual to Resource properties - AnnotationDeployer reads virtual() from @Managed*Definition annotations Tests skip gracefully on Java <21 via Assume.assumeTrue.
- ScheduleHelper: remove pass-through toMonths/toDaysOfWeek methods and unused DayOfWeek/Month imports - ManagedScheduledExecutorServiceImplFactory: wire virtual flag into fallback thread factory creation, remove unused import - VirtualThreadHelper: remove unused ofVirtualInterface variable
…duler Verify that API-provided CronTrigger (a ZonedTrigger) works transparently with ManagedScheduledExecutorServiceImpl via default bridge methods. Tests confirm recurring scheduling and LastExecution with ZonedDateTime work without any implementation changes.
The TCK beans call Asynchronous.Result.getFuture() inside scheduled methods. The interceptor must call setFuture() before ctx.proceed() for both void and non-void return types, otherwise getFuture() throws IllegalStateException. Use Callable path for all scheduled methods to ensure proper future lifecycle.
…d JNDI Three fixes for @asynchronous(runAt=@schedule(...)) TCK compliance: 1. Stop trigger loop when method returns non-null value (per spec: "the method returns a non-null result value" ends the schedule). Uses AtomicReference to cancel ScheduledFuture from inside Callable. 2. Throw IllegalArgumentException for invalid executor JNDI names instead of silently falling back to default. Only default names get the graceful fallback. 3. Add beans.xml archive processor for Concurrency TCK deployments (OWB needs bean-discovery-mode="all" to auto-enable @priority interceptors). 4. Add TCK-style unit test covering CompletedResult, IncompleteFuture, CompletedExceptionally, VoidReturn, and InvalidJNDIName scenarios.
bean-discovery-mode=all caused deployment failures for some TCK WARs by scanning too many classes. Switch to annotated mode which is the CDI 4.0 default and sufficient for discovering annotated beans.
Extend TCK-style test coverage with multipleSchedules (composite trigger with two @schedule annotations) and ignoresMaxAsync (concurrent scheduled method invocations). Both pass — remaining TCK failures for these tests are JNDI scoping issues with java:module/ executor names.
…tors Strip java: prefix in ManagedScheduledExecutorServiceImplFactory.lookup() fallback path to match how resources are registered via cleanUpName(). Without this, java:module/concurrent/ScheduledExecutorB lookups fail because the resource is bound as module/concurrent/ScheduledExecutorB. Add Arquillian test verifying both java:module/ and java:app/ scoped @ManagedScheduledExecutorDefinition work with scheduled async methods.
Per spec, @asynchronous(executor=..., runAt=@schedule(...)) may reference a plain ManagedExecutorService, not just a ManagedScheduledExecutorService. When MSES lookup fails, verify the executor exists as MES and fall back to the default MSES for scheduling capability.
…tion Two fixes for the last Web-profile TCK failures: 1. Context propagation: when executor is a plain MES (not MSES), extract the MES's ContextServiceImpl and compose a temporary MSES that uses the MES's context service with the default MSES's thread pool. This preserves third-party context propagation (e.g. StringContext). 2. Thread pool starvation: replace mses.schedule(Callable, Trigger) with a manual trigger loop that schedules directly on the delegate ScheduledExecutorService. This avoids TriggerTask's double context wrapping and thread consumption between trigger fires.
Per spec, scheduled async methods are not subject to maxAsync constraints. Use the default MSES's thread pool for the trigger loop instead of the referenced executor's pool. This prevents thread starvation when maxAsync threads are busy with blocking tasks.
- Add qualifier field (List<String>) to ContextService, ManagedExecutor, ManagedScheduledExecutor, ManagedThreadFactory DD model classes - Update SXC JAXB accessors to parse <qualifier> and <virtual> XML elements (virtual was in the model but missing from SXC parsers) - Fix NPE in Convert*Definitions when <context-service-ref> is absent in deployment descriptor — defaults to java:comp/DefaultContextService - Add unit tests for null context service fallback and qualifier model - Add Arquillian test deploying WAR with web.xml containing <virtual>
ForkJoinWorkerThread extends Thread (platform) and cannot be virtual. Fall back to a platform ManagedForkJoinWorkerThread instead of throwing UnsupportedOperationException. The TCK expects ForkJoinPool to function with virtual factories — the worker threads are platform but the pool still operates correctly. m>
Register concurrency resources (ManagedExecutorService, ManagedScheduledExecutorService, ManagedThreadFactory, ContextService) as CDI beans with qualifier support per Concurrency 3.1 spec Section 5.4.1. - ConcurrencyCDIExtension: CDI extension that observes AfterBeanDiscovery and creates synthetic ApplicationScoped beans for resources with qualifiers. Also registers default beans (@Default/@Any) for all four concurrency resource types. - AnnotationDeployer: Extract qualifiers() from @ManagedExecutorDefinition, @ManagedScheduledExecutorDefinition, @ManagedThreadFactoryDefinition, @ContextServiceDefinition annotations into JEE model objects. - Convert*Definitions: Pass Qualifiers as comma-separated Resource property. - OptimizedLoaderService: Register ConcurrencyCDIExtension alongside JMS2CDIExtension. TCK Web profile: 196/196 passing (0 failures, 0 errors).
Per Concurrency 3.1 spec: "When running on Java SE 17, the true value behaves the same as the false value and results in platform threads being created rather than virtual threads." Previously, virtual=true unconditionally called VirtualThreadHelper methods which throw UnsupportedOperationException on Java 17. Now checks VirtualThreadHelper.isSupported() first and falls through to platform thread creation when virtual threads are unavailable.
…e headers Replace ctx.proceed() with direct Method.invoke() in scheduled async re-executions. InvocationContext's interceptor iterator is single-use, so re-calling proceed() would bypass TX/security interceptors after the first scheduled execution. Context propagation is handled by ContextService.enter/exit. Also add Apache License headers to TCK resource files to fix RAT check.
Re-dispatch each scheduled firing through the CDI interceptor proxy so TX, security, and application interceptors run on every run. The prior code called beanMethod.invoke(target, ...) directly and bypassed the chain; InvocationContext.proceed() cannot be reused because OWB's implementation advances an index that never resets, so a second call falls through to invoking the target method and skips every lower- priority interceptor. OWB coupling is isolated in ScheduledAsyncInvoker, which reuses the existing OwbInterceptorProxy when the bean is normal-scoped and otherwise builds a fresh interceptor proxy via InterceptorResolutionService.createProxiedInstance. Re-entry detection uses a precise (method, target) key, restored rather than cleared on exit so nested @asynchronous calls on different methods or targets do not collide. Also routes scheduled firings to the executor named in @asynchronous(executor=...) via mses.getDelegate() rather than silently switching to DefaultManagedScheduledExecutorService, so custom thread factories, priorities, and virtual-thread settings take effect. Adds tests for interceptor ordering across firings and for routing the firing thread to the requested executor vs. the default pool.
ManagedScheduledExecutorServiceImplFactory used to resolve its thread factory via ThreadFactories.findThreadFactory, which calls the no-arg ManagedThreadFactoryImpl constructor reflectively. That constructor hard-codes virtual=false, so @ManagedScheduledExecutorDefinition( virtual=true) was silently ignored unless the reflective lookup failed and the catch block instantiated the factory directly. Short-circuit the default class name to construct the factory with the configured virtual flag, matching the pattern already used in ManagedExecutorServiceImplFactory. Adds regression tests asserting that virtual=true on the scheduled factory yields threads that do not implement ManageableThread (per Concurrency 3.1 §3.4.4) and that the default path still produces platform threads.
Introduces @EnabledForJreRange plus a small JreConditionRule so JUnit 4 tests can declare a JRE feature-version range at the method level, mirroring the semantics of JUnit Jupiter's EnabledForJreRange. The rule reads the annotation from the test description and throws AssumptionViolatedException when the current JRE is outside the range, so Surefire reports out-of-range methods as skipped. Migrates VirtualThreadHelperTest off imperative Assume.assumeTrue / Assume.assumeFalse calls: Java 21+ cases use @EnabledForJreRange(min=21), the UnsupportedOperationException cases use @EnabledForJreRange(max=20).
Mirrors the failing TCK scenario ManagedScheduledExecutorDefinitionWebTests. testScheduledAsynchIgnoresMaxAsync in-process via ApplicationComposer: a custom ManagedScheduledExecutorService with maxAsync=1 is saturated with a blocking submit, then an @asynchronous(runAt=@schedule) method targeting that executor is invoked. Per Concurrency 3.1 the scheduled firing must not be subject to maxAsync, but after routing scheduled firings onto mses.getDelegate() the firing is queued behind the saturating task because the delegate's ScheduledThreadPoolExecutor has corePoolSize = maxAsync.
Add a secondary ScheduledExecutorService to every ManagedScheduledExecutorServiceImpl, dedicated to dispatching @asynchronous(runAt=@schedule(...)) firings. The secondary pool shares the primary's ManagedThreadFactory (preserving naming, virtual flag, priority, and context) but is sized independently via a new ScheduledAsyncCore resource property (default 5). The interceptor now triggers on the secondary pool, so scheduled firings are no longer throttled by the primary pool's corePoolSize=maxAsync limit — satisfying Concurrency 3.1 §3.1 "Scheduled asynchronous methods are treated similar to other scheduled tasks in that they are not subject to max-async constraints" without losing the clause from PR #2577 review that firings must run on the executor named in @asynchronous(executor=). The plain-MES fallback wrapper borrows the default MSES's secondary so short-lived per-invocation wrappers do not leak a fresh pool each time; an ownership flag on the 4-arg constructor controls which pools destroyResource() shuts down. Also fix a pre-existing race in CUTriggerScheduledFuture: cancel() can race ahead of the recursive scheduleNextRun(), leaving futureRef pointing at the just-completed execution so the underlying future reports done rather than cancelled. isCancelled() now falls back to the TriggerTask's cancelled flag. Tests: - ScheduledAsyncCustomFactoryTest — scheduled firing runs on a ManageableThread from the custom MSES's factory, not the primary pool. - ManagedScheduledExecutorSecondaryPoolLifecycleTest — owned secondary is shut down on destroyResource(); borrowed secondary is not. TCK Web profile back to 196/0/0/14.
AnnotationDeployer#buildManaged{Executor,ScheduledExecutor,ThreadFactory}Definition
was unconditionally overwriting deployment-descriptor values (name,
context, hungTaskThreshold, maxAsync, virtual, priority) with annotation
values whenever both sources shared a resource name. That violates the
Jakarta EE platform DD-wins rule (§EE.5.2.5); only qualifier was guarded.
Guard every field assignment with a null check, mirroring the existing
buildContextServiceDefinition pattern, so DD values are preserved and the
annotation only fills attributes the descriptor left unset.
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
2 participants
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.
First version - think we can improve or re-work if needed.