984470 - When the profiler stops it doesn't stop the EventTracer thread on Windows

Status: RESOLVED WONTFIX

(Core :: Gecko Profiler, defect)

Description

[Viktor Stanchev [:vikstrous]]

This is a follow up to Bug 965373 - When the profiler stops it doesn't stop the EventTracer thread

Relevant info:

> I had to disable this feature for Windows because it was causing a test to
> fail in very strange ways.
> 
> Originally we had a deadlock when starting and stopping the event tracer
> within the same block of javascript code on Windows. It was caused by the
> code to get a pointer to the hidden window. It had to dispatch an event to
> the main thread to do so.
> 
> This was done on the first call of FireAndWaitForTracerEvent. I tried moving
> the call to GetHiddenWindowHWND from there to InitWidgetTracing. This fixed
> the deadlock (this was done in a previous version of my patch). However,
> this made this test fail:
> toolkit\devtools\server\tests\unit\test_profiler_actor.js
> 
> I couldn't reproduce this new bug in an actual browser.
> 
> The test starts the sampler in jankOnly mode. It uses the remote debugging
> feature. The test fails because no samples are collected. There is no
> deadlock as far as I know. The sampler simply does not record any samples in
> jankOnly mode. I'm not entirely sure what's causing the issue. My theory was
> that it was getting the wrong HWND handle, but since I couldn't reproduce
> the issue in an actual browser, I don't know if that's the case. It might be
> an issue with the way the test is run. This requires much more effort than
> we expected to fix, so as a temporary workaround I just made sure my patch
> doesn't turn off the event tracer on Windows.

Comment 1

[:Harald Kirschner :digitarald]

:mstange mentioned that this might not be used anymore but the code still exists. Should we close or update this bug?

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

I doubt it is still used, as trying MOZ_INSTRUMENT_EVENT_LOOP=1 ./mach run crashes quickly (nsThread assertions in the GPU process). The code seems to date from 2011-2014.

So I'd say this specific bug is not useful anymore.

We have a thread-responsiveness checker in the Gecko Profiler.
So the question is: Do we still want or need this EventTracer? (Which I guess gave more details about events.)

I'll close this bug as WONTFIX, assuming we won't attempt to revive the EventTracer.