client: do not fork a child process unless a tunnel is configured - #1604
Open
bpalermo wants to merge 1 commit into
Open
client: do not fork a child process unless a tunnel is configured#1604bpalermo wants to merge 1 commit into
bpalermo wants to merge 1 commit into
Conversation
ProcessImpl::run() always went through EncapsulationSubProcessRunner, forking a child even without --tunnel-uri; in that case the child only posts a process-shared semaphore and exits while the parent blocks in sem_wait(). The parent is already multithreaded at that point (the signal handler thread, allocator state), so the child could deadlock on a lock owned by a thread that does not exist in it before ever reaching sem_post(), leaving nighthawk_client hung at startup with no request sent. Run the load in-process unless a tunnel is actually requested. Signed-off-by: Bruno Palermo <[email protected]>
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
1 participant
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.
Description
This PR is related to #1609
ProcessImpl::run()always goes throughEncapsulationSubProcessRunner, whichfork()s a child even when no--tunnel-uriis configured. In that case the child does nothing butsem_post()a process-shared semaphore and_exit(), while the parent blocks insem_wait()until it does. The parent is already multithreaded at that point (at least theSignalHandlerthread; in-c optbuilds also the allocator's state), so the child can deadlock on a lock owned by a thread that does not exist in it before it ever reachessem_post(), and the client hangs at startup without sending a request. This runs the load in-process unless a tunnel is actually requested; the fork is still used for the tunneling path, where the child runs the encapsulating Envoy.Notes for Reviewers
encap_runner_, so no other change is needed.//test:process_testand//test:python_test(which includes the tunneling integration tests) pass. Version history updated.