Fix functionInstanceInjector null on concurrent cold start (#879)#880
Merged
ahmedmuhsin merged 1 commit intoJul 23, 2026
Merged
Conversation
initializeOneTimeLogics set oneTimeLogicInitialized to true before initializeFunctionInstanceInjector ran. The outer flag check is lock-free, so a second load thread could observe the flag as set, skip init, and invoke while functionInstanceInjector was still null. That failed every invocation on the affected instance with an NPE in ExecutionContextDataSource.getFunctionInstance. Restore the ordering from Azure#684 that Azure#819 undid, so the injector is built before the flag flips. Add a regression test that freezes a thread at injector init and checks the flag stays false until init completes.
ahmedmuhsin
marked this pull request as ready for review
July 22, 2026 03:27
ahmedmuhsin
requested review from
TsuyoshiUshio,
gavin-aguiar,
hallvictoria,
swapnil-nagar and
vrdmr
as code owners
July 22, 2026 03:27
TsuyoshiUshio
approved these changes
Jul 23, 2026
TsuyoshiUshio
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
One tip for CountDownLatch If it works, that's great, but when I use similar solution in C#, CI has very few thread, so that blocked long and eventually timed out. But It is also possible not the case. Let's merge it and see if it works. If it works, I don't have any objection and CountDownLatch will remove flaky testing.
swapnil-nagar
approved these changes
Jul 23, 2026
ahmedmuhsin
added a commit
that referenced
this pull request
Jul 24, 2026
initializeOneTimeLogics set oneTimeLogicInitialized to true before initializeFunctionInstanceInjector ran. The outer flag check is lock-free, so a second load thread could observe the flag as set, skip init, and invoke while functionInstanceInjector was still null. That failed every invocation on the affected instance with an NPE in ExecutionContextDataSource.getFunctionInstance. Restore the ordering from #684 that #819 undid, so the injector is built before the flag flips. Add a regression test that freezes a thread at injector init and checks the flag stays false until init completes.
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.
What this fixes
Fixes #879. Since worker 2.19.2, an instance can start up with
functionInstanceInjectorleft null for its whole lifetime, and then every invocation on that instance fails with an NPE.Root cause
JavaFunctionBroker.initializeOneTimeLogics()uses double-checked locking behind the volatileoneTimeLogicInitializedflag. Since 2.19.2 it set the flag to true before building the injector.Function load requests are dispatched concurrently, since
JavaWorkerClientsubmits every message to a cached thread pool. At cold start two threads runloadMethodat the same time. Thread A enters the locked block and sets the flag. Thread B hits the lock-free outer check, sees the flag already true, returns early, and starts invoking while the injector is still null. TheServiceLoaderscan for a custom injector (for example the Spring Cloud Function adapter) widens the window.This is the same NPE that #684 fixed. #819 (OpenTelemetry support) reordered the two lines and reopened it.
The fix
Build the injector before flipping the flag, which restores the #684 ordering.
Test
Adds
JavaFunctionBrokerConcurrentInitTest, which freezes a thread at the point the injector is about to be built and assertsoneTimeLogicInitializedis still false there. It fails on the old ordering and passes with the fix.Affected versions
Regressed in 2.19.2 and present through 2.19.4. 2.19.1 and earlier are fine.