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 <bruno.palermo@superbid.net>
Brings the branch up to date with the Envoy bump to 82c182e, the envoy_package additions, envoyproxy#1598 and envoyproxy#1599. The only conflict was the version history changelist, where both sides add a bullet; both are kept. Signed-off-by: Bruno Palermo <bruno.palermo@superbid.net>
eric846
requested changes
Sep 17, 2026
eric846
left a comment
Contributor
There was a problem hiding this comment.
This seemed to fail the tsan test in the CI, in a Python integration test.
Could you see if you can reproduce any failure like that locally with ci/do_ci.sh tsan? It should also fail on your machine if there's a real issue.
It's also possible for the CI to be flaky, like if the VM used in the CI is slow. In that case we would have to look further into the test and possibly adjust some numbers to reduce the flakiness.
Brings the branch up to date after envoyproxy#1601 landed. The only conflict was the version history changelist, where both sides add a bullet; both are kept. Pushing this also gives the tsan job a second run: the failure eric846 saw was test_https_prefetching, which asserts 50 TLS connections are established inside a 1 second run, and it failed because the counter was absent rather than wrong, i.e. no connection completed in time. Signed-off-by: Bruno Palermo <bruno.palermo@superbid.net>
No code change. The tsan job has failed in a different wall-clock sensitive Python integration test on each run (test_https_prefetching, then test_linear_ramping_rate_limiter_plugin), and upstream main failed test_tracing_zipkin on a Dockerfile base image bump, so this samples the job once more. Signed-off-by: Bruno Palermo <bruno.palermo@superbid.net>
Contributor
Author
|
@eric846 indeed seems it's a flaky test. |
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.
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.