It is strange that there are multiple sensor threads. This component should only be creating one at most.

The separate thread is used to execute potentially blocking tasks and is also where COM callbacks for sensor data are delivered. Does the common pool support COM in this way?

gab@, what is the source for this data as well?

The source is etienneb@'s traces from the wild (from Slow Reports).

@robliao : does our pool support COM callbacks? (i.e. we support COM usage but do we process callbacks? don't think so?)

Here are some example from slow-reports:

  Report ID       Chrome Version  # Threads
9f2aabcd636c2163    71.0.3551.3   115
e9ca9e53967bae0e    71.0.3551.3   115
5234aebc99850f23    71.0.3551.3   114
2176739a6c3ade41    71.0.3550.3   109
8c12238faa17e263    71.0.3550.3   109
097c756616498269    71.0.3551.3   108
03b56e6e0919175f    71.0.3550.3   107
ae69081e5b31d983    71.0.3551.3   106
86e633fa91f4a49f    71.0.3551.3   103
8881bfa8ebd81041    71.0.3552.3    98
7a557f4e0652eb92    71.0.3551.3    97
00a009bc6afd9157    71.0.3551.3    96

See attachment, from this 9f2aabcd636c2163 (115 threads)

Deleted: sensors.png 159 KB

	sensors.png 159 KB View Download

It would definitely be a possible explanation for issue 789344 if somehow this singleton base::Thread instance were being started multiple times. Every time I've skimmed through platform_sensor_provider_win.cc I haven't been able to come up with an explanation for how there could be multiple instances.

Our COM STA single thread task runners, both shared and dedicated, support callbacks by pumping Windows messages.

For MTAs, any MTA thread can access any object. This means that there is no expectation that an MTA pumps messages. My understanding here is that COM will dispatch any remote callback through one of its worker threads in the process.
The only requirement is that the component receiving the callback needs to be alive.

*Any MTA thread can access any MTA object.

Would a rapid succession of CreateSensor() and FreeResources() be possible? That seems like it might create a bunch of threads.

Do you mean
https://cs.chromium.org/chromium/src/services/device/generic_sensor/platform_sensor_provider_win.cc?l=118
can be called multiple times in this case?

Right. Calling StartSensorThread() multiple times is not a problem in and of itself. However, if you call StartSensorThread() with FreeResources() in succession, that can create a lot of threads.

So if this is possible (I haven't checked the FreeResources() codepath closely), then this is possibly how it happened.

Just want to make sure I understand, does it mean even though there is only one instance of |sensor_thread_|, calling "sensor_thread_->Start()" multiple times can create a lot of Sensor threads?

While there is only one instance of |sensor_thread_|, that doesn't mean it only mean one operating system thread ever allocated to it.

It's important to separate the behavior of StartSensorThread() and the #12's sensor_thread_->Start() expressions.

It is an error to call base::Thread::Start() while a thread is already running. It will DCHECK that the thread isn't already running. StartSensorThread() avoids the DCHECK by using a sensor_thread_->IsRunning() check.

Now let's look at Start() and Stop() mechanics of base::Thread(). Calling Start() on a thread does the actual OS request to create and start the thread. A corresponding call to Stop() will stop and _destroy the OS thread_. So while you maintain a single instead to a base::Thread, base::Thread can create and destroy an OS thread as needed.

As a result, calls to Start() and Stop() in succession will create and destroy many OS threads. These threads don't run for a long time, but they still show up as a line item in the Chrome trace.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f62f3622b59a1b9b5a61097669cef6fea851b3a

commit 8f62f3622b59a1b9b5a61097669cef6fea851b3a
Author: Jun Cai <juncai@chromium.org>
Date: Tue Oct 02 21:10:16 2018

Sensor API: Remove SensorThread from platform_sensor_provider_win.cc

This CL removes SensorThread from platform_sensor_provider_win.cc and
uses the //base/task/post_task.h APIs instead.

Bug:  883877 
Change-Id: Icd3632f33e58b2577c2f2d6925d9ce5c3469e4d0
Reviewed-on: https://chromium-review.googlesource.com/c/1246063
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595980}
[modify] https://crrev.com/8f62f3622b59a1b9b5a61097669cef6fea851b3a/services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
[modify] https://crrev.com/8f62f3622b59a1b9b5a61097669cef6fea851b3a/services/device/generic_sensor/platform_sensor_provider_win.cc
[modify] https://crrev.com/8f62f3622b59a1b9b5a61097669cef6fea851b3a/services/device/generic_sensor/platform_sensor_provider_win.h