mobile: support handling requests on worker thread by danzh2010 · Pull Request #44295 · envoyproxy/envoy

danzh2010 · 2026-04-06T21:25:08Z

Commit Message: add a config knob threading_model in ApiListenerManager. And add an interface enableWorkerThread() in C++ and python EngineBuilder to allow choosing a threading model for mobile engine. In addition to the exiting MAIN_THREAD_ONLY model mostly used for mobile deployment, this PR introduces a STANDALONE_WORKER_THREAD model which will drive http requests in a standalone worker thread. This allows E-M to be more like Envoy which also run HTTP listeners and xDS processing on different threads so that network events won't be blocked by xDS config updates.

Note that in STANDALONE_WORKER_THREAD mode certificates validation offloading and system proxy setting is NOT supported as this threading model is not meant to be used by mobile applications.

Risk Level: low, behind knobs
Testing: existing integration tests pass
Docs Changes: N/A
Release Notes: N/A

…pdate tests Signed-off-by: Dan Zhang <danzh@google.com>

Signed-off-by: Dan Zhang <danzh@google.com>

…performance Signed-off-by: Dan Zhang <danzh@google.com>

Signed-off-by: Dan Zhang <danzh@google.com>

…ly and update tests' Signed-off-by: Dan Zhang <danzh@google.com>

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only · 2026-04-06T21:25:19Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #44295 was opened by danzh2010.

see: more, trace.

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2026-04-06T21:42:00Z

/assign @paul-r-gall

Signed-off-by: Dan Zhang <danzh@google.com>

… of client_. Signed-off-by: Dan Zhang <danzh@google.com>

…Change ASSERT to RELEASE_ASSERT in ApiListenerManagerImpl::httpClient() to ensure it's checked in opt builds.\n- Add info log when worker thread is started.\n- Comment out trace log level setting in rtds_integration_test.cc.\n- Format client_test.cc. Signed-off-by: Dan Zhang <danzh@google.com>

markdroth · 2026-04-08T21:56:34Z

@@ -458,4 +458,14 @@ message ValidationListenerManager {
 // Listener Manager via the bootstrap's :ref:`listener_manager <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.listener_manager>`.
 // [#not-implemented-hide:]
 message ApiListenerManager {


Why is this message in this file to begin with, rather than being in bootstrap.proto? It seems like this wouldn't ever be used outside of the bootstrap, so it doesn't really belong with the rest of the xDS API.

My guess is that all the listener manager extensions go into this file.

markdroth · 2026-04-08T21:57:07Z

 message ApiListenerManager {
+  enum ThreadingModel {
+    // Handle HTTP requests on the main Envoy thread which also processes platform-raised events and runs xDS clients.
+    MainThreadOnly = 0;


Protobuf enum elements should be in all-caps, like MAIN_THREAD_ONLY.

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2026-04-09T18:33:24Z

/retest

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2026-04-09T20:59:06Z

This is ready for review!
/assign @abeyad

phlax · 2026-04-14T12:30:08Z

@abeyad i think this is waiting on your sign off

danzh2010 · 2026-04-14T13:58:58Z

friendly ping?

abeyad

thanks @danzh2010 !

abeyad · 2026-04-10T17:11:25Z

  return *this;
 }

+EngineBuilder& EngineBuilder::enableEarlyData(bool early_data_on) {


why are early data changes in this PR?

Some tests accidentally enable early data due to turning off cert validation offloading in some test combination. And early data request doesn't have ssl_start_time populated in final_stream_intel which breaks test assumption. But I figured out another way to bypass the validation. So I reverted early data related change.

abeyad · 2026-04-14T13:57:35Z


 #if defined(__APPLE__)
-  if (respect_system_proxy_settings_) {
+  if (respect_system_proxy_settings_ && !use_worker_thread_) {


are we using "no separate worker thread" here to mean "not mobile"? I think that would be a dangerous assumption. For non-mobile use cases, respect_system_proxy_settings_ should be set to false anyway.

"no separate worker thread" means "mobile". Or I should say we only allow respect_system_proxy_settings_ when "no separate worker thread" otherwise the knob is no-op.

I'm not sure why we check both here though. We should only check respect_system_proxy_settings_. If we think it's related, when we set "enableStandaloneWorkerThread()", we should also set respect_system_proxy_settings_ to false there, but I don't even think we need to do that. Unless I'm misunderstanding?

The check here just enforces these 2 knobs won't take effect at the same time if a user misconfigured the builder. The knobs can be called in any sequence on the builder, so if we want to enforce this in the setter, we need to do it in both setters. I slightly prefer adding some enforcement somehow otherwise the Engine can still run but will run into data race?

WDYT on this?

I agree with preventing the system proxy settings from being invoked unless it's on mobile. I'm concerned that the code is very confusing as to why it checks if a worker thread is enabled or not when deciding whether to register the apple system proxy settings.

Ideally, this would be solved by different EngineBuilders for Envoy "Client" vs. Envoy "Mobile", that can enforce the various constraints for the mobile vs. non-mobile use case. There's already an ifdef guard for __APPLE__ guarding this code, but I suppose that would still execute on MacOS and other non-iOS Apple devices.

Also, ideally, EngineBuilder::build() would return StatusOr<EngineSharedPtr> instead of just EngineSharedPtr. That way, we could return a status error if the config is in a logically inconsistent state. But that would be a separate change.

For now, I'm OK with keeping this check as long as we write clear code comments explaining why we are also checking for !use_worker_thread_

I moved the enforcement to both enableWorkerThread() and respectSystemProxySetings() so that the former takes precedence. Likewise for platform cert validation. PTAL.

abeyad · 2026-04-14T14:00:45Z

  EngineBuilder& enableBrotliDecompression(bool brotli_decompression_on);
  EngineBuilder& enableSocketTagging(bool socket_tagging_on);
  EngineBuilder& enableHttp3(bool http3_on);
+  EngineBuilder& setUseWorkerThread(bool use_worker_thread);


How about enableStandaloneWorkerThread? We should also add comments to clarify what this method does i.e. what a separate worker/network thread means

abeyad · 2026-04-14T14:08:39Z

+  worker_started_notification.WaitForNotification();
+  ENVOY_LOG_MISC(info, "Worker thread has been started");
+
+  worker_->httpClient(); // Verify if it's null right after startup


what does this comment mean here?

httpClient() ASSERT on client_ to be not null.

Signed-off-by: Dan Zhang <danzh@google.com>

…ker thread Signed-off-by: Dan Zhang <danzh@google.com>

abeyad

thanks @danzh2010 !

abeyad

@markdroth friendly ping on the API review, thanks!

Signed-off-by: Dan Zhang <danzh@google.com>

danzh1989 added 11 commits March 30, 2026 17:38

Refactor ConnectivityManager to take DnsCacheSharedPtr directly and u…

3d7b609

…pdate tests Signed-off-by: Dan Zhang <danzh@google.com>

Checkpoint of remaining uncommitted changes

5bf916c

Signed-off-by: Dan Zhang <danzh@google.com>

Clean up debug logs in client.cc and update assert messages

0e8ec33

Signed-off-by: Dan Zhang <danzh@google.com>

Remove BACKTRACE_LOG from api_listener_manager for better production …

4972835

…performance Signed-off-by: Dan Zhang <danzh@google.com>

Generic plumbing for createListenerManager with empty config

fd52696

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'listenermanager' into mobileworkerthread2

3d73f77

Signed-off-by: Dan Zhang <danzh@google.com>

Parameterize ClientIntegrationTest for threading toggles

00fd465

Signed-off-by: Dan Zhang <danzh@google.com>

Revert 'Refactor ConnectivityManager to take DnsCacheSharedPtr direct…

a2535e4

…ly and update tests' Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into mobileworkerthread2

b0f5f24

Signed-off-by: Dan Zhang <danzh@google.com>

add a knob and integration tests

74406d9

Signed-off-by: Dan Zhang <danzh@google.com>

refine code

d173884

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 requested review from RyanTheOptimist and abeyad as code owners April 6, 2026 21:25

repokitteh-read-only bot added the api label Apr 6, 2026

repokitteh-read-only bot assigned markdroth Apr 6, 2026

danzh2010 changed the title ~~mobile: support running requests on worker thread~~ mobile: support handling requests on worker thread Apr 6, 2026

add python binding

5310ad3

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot assigned paul-r-gall Apr 6, 2026

danzh1989 added 3 commits April 8, 2026 18:03

mobile: fix thread safety and lifecycle issues in ApiListenerManager

0b3f15b

Signed-off-by: Dan Zhang <danzh@google.com>

Fix compilation error in client_test.cc by using http_client_ instead…

632aae0

… of client_. Signed-off-by: Dan Zhang <danzh@google.com>

markdroth reviewed Apr 8, 2026

View reviewed changes

use listener hooks

2730ec9

Signed-off-by: Dan Zhang <danzh@google.com>

danzh1989 added 2 commits April 9, 2026 18:45

Change enum values to uppercase and add const

dc0f900

Signed-off-by: Dan Zhang <danzh@google.com>

Merge remote-tracking branch 'upstream/main' into mobileworkerthread2

3125f92

Signed-off-by: Dan Zhang <danzh@google.com>

repokitteh-read-only bot assigned abeyad Apr 9, 2026

abeyad reviewed Apr 14, 2026

View reviewed changes

danzh1989 added 3 commits April 15, 2026 15:18

address comment

a000d73

Signed-off-by: Dan Zhang <danzh@google.com>

fix build error

dc2dd09

Signed-off-by: Dan Zhang <danzh@google.com>

no system proxy settings and platform cert validation while using wor…

25bb0a2

…ker thread Signed-off-by: Dan Zhang <danzh@google.com>

abeyad previously approved these changes Apr 16, 2026

View reviewed changes

abeyad reviewed Apr 16, 2026

View reviewed changes

Refine worker thread termination and add logging

10923ea

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 dismissed abeyad’s stale review via 10923ea April 16, 2026 17:40

Merge branch 'main' into mobileworkerthread2

552cdef

Signed-off-by: Dan Zhang <danzh@google.com>

Conversation

danzh2010 commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Apr 6, 2026

Uh oh!

danzh2010 commented Apr 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danzh2010 commented Apr 9, 2026

Uh oh!

danzh2010 commented Apr 9, 2026

Uh oh!

phlax commented Apr 14, 2026

Uh oh!

danzh2010 commented Apr 14, 2026

Uh oh!

abeyad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abeyad left a comment

Choose a reason for hiding this comment

Uh oh!

abeyad left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

danzh2010 commented Apr 6, 2026 •

edited

Loading