This changes comes with several different refactorings all rolled into one,
unfotunately I couldn't find a way to pull them apart:
- First of all annotations now can either recorded (that is, we copy the value
and have the crash reporting code own the copy) or registered. Several
annotations are changed to use this functionality so that we don't need to
update them as their value change.
- The code in the exception handler is modified to read the annotations from
the mozannotation_client crate. This has the unfortunate side-effect that
we need three different bits of code to serialize them: one for annotations
read from a child process, one for reading annotations from the main process
outside of the exception handler and one for reading annotations from the
main process within the exception handler. As we move to fully
out-of-process crash reporting the last two methods will go away.
- The mozannotation_client crate now doesn't record annotation types anymore.
I realized as I was working on this that storing types at runtime has two
issues: the first one is that buggy code might change the type of an
annotation (that is record it under two different types at two different
moments), the second issue is that types might become corrupt during a
crash, so better enforce them at annotation-writing time. The end result is
that the mozannotation_* crates now only store byte buffers, track the
format the data is stored in (null-terminated string, fixed size buffer,
etc...) but not the type of data each annotation is supposed to contain.
- Which brings us to the next change: concrete types for annotations are now
enforced when they're written out. If an annotation doesn't match the
expected type it's skipped. Storing an annotation with the wrong type will
also trigger an assertion in debug builds.
Differential Revision: https://phabricator.services.mozilla.com/D195248
This both reduces the number of registered static destructors, and avoids the
assertion in the LinkedList destructor without depending on a StaticAutoPointer
(which requires an allocation & additional indirection) by using the new
NeverDestroyed type from part 4.
Differential Revision: https://phabricator.services.mozilla.com/D189249
This fixes the issue described in comment 7 by doing a few changes to how threads are created, and members on nsThreadManager are managed.
* The thread list is moved to be a member of `nsThreadManager`, which is
already a statically allocated singleton.
* The flag for whether new thread creation is allowed is moved from being an
atomic to being guarded by the same mutex as the thread list, meaning that
it could be checked and a new thread could be added to the list atomically.
* Threads which require shutdown are initialized atomically from the point of
view of the thread manager, meaning that the check in ShutdownNonMainThreads
is guaranteed to catch all successfully started nsThread instances.
Differential Revision: https://phabricator.services.mozilla.com/D188623
This fixes the issue described in comment 7 by doing a few changes to how threads are created, and members on nsThreadManager are managed.
* The thread list is moved to be a member of `nsThreadManager`, which is
already a statically allocated singleton.
* The flag for whether new thread creation is allowed is moved from being an
atomic to being guarded by the same mutex as the thread list, meaning that
it could be checked and a new thread could be added to the list atomically.
* Threads which require shutdown are initialized atomically from the point of
view of the thread manager, meaning that the check in ShutdownNonMainThreads
is guaranteed to catch all successfully started nsThread instances.
Differential Revision: https://phabricator.services.mozilla.com/D188623
This will be used to deprioritize whole thread pools. The concept works as such:
- Set a thread pool's priority at any time
- Each thread will ensure their priority mirrors the parent pool's priority when it is finished processing events.
I chose to have threads only set themselves to the background when they finish processing events because an event dispatched at an expected priority should be completed at said priority. This ensures that any event dispatch that may expect normal priority (i.e. not operating in the background) has a chance to run at the established priority.
This patch enrolls the background IO to low priority on its creation. It also includes the MacOS implementation of the thread priorities, which utilizes mac's QoS service. According to MacOS's QoS structure, main threads operate at `user-interactive` and offthread work is done at a `default` state, meaning that the OS decides how to set the thread's priority somewhere between `utility` and `user-interactive`. When a thread is put in the background, it must be manually raised back to the expected priority, which means most threads must set themselves to `default` again and the main thread must set itself to `user-interactive`. I demonstrated this in the `nsThread` implementation by checking for the main thread and if this task is running on the main thread somehow, then we use the correct setting.
Rather than having the thread pools dispatch their own deprioritization tasks, I've decided to make this the job of the nsThread. In following bugs we will put entire processes' nsThreads in the background, which will require more use of the new nsThread codepaths.
This behavior is currently restricted to nightly only. On platforms with no implementation, nothing will change.
Differential Revision: https://phabricator.services.mozilla.com/D146534
This was always being tracked internlly to implement
nsThreadPool::IsOnCurrentThread, however this patch exposes the getter
publicly.
This will be used to assert if GetCurrentSerialEventTarget() is called from a
threadpool thread without a TaskQueue in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D166603
This was always being tracked internlly to implement
nsThreadPool::IsOnCurrentThread, however this patch exposes the getter
publicly.
This will be used to assert if GetCurrentSerialEventTarget() is called from a
threadpool thread without a TaskQueue in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D166603
Some debug infrastructure like MOZ_WEAKPTR_INIT_THREAD_SAFETY_CHECK that can apparently be triggered by nsComponentManagerImpl::gComponentManager->FreeService() seem to rely on the existence of a serial event target even if they do not post any events. So it seems sound to keep a representation of the main thread as nsThread object until after final XPCOM shutdown.
But nsThreadManager::ShutdownMainThread() does more, it processes the last round of events, removes the thread's observer and closes down the background event target. We do not want to move these operations to happen later than before, such that we split the nsThread release into a separate function and move that together with AbstractThread::ShutdownMainThread() behind FreeService().
Depends on D160628
Differential Revision: https://phabricator.services.mozilla.com/D162497
Some debug infrastructure like MOZ_WEAKPTR_INIT_THREAD_SAFETY_CHECK that can apparently be triggered by nsComponentManagerImpl::gComponentManager->FreeService() seem to rely on the existence of a serial event target even if they do not post any events. So it seems sound to keep a representation of the main thread as nsThread object until after final XPCOM shutdown.
But nsThreadManager::ShutdownMainThread() does more, it processes the last round of events, removes the thread's observer and closes down the background event target. We do not want to move these operations to happen later than before, such that we split the nsThread release into a separate function and move that together with AbstractThread::ShutdownMainThread() behind FreeService().
Depends on D160628
Differential Revision: https://phabricator.services.mozilla.com/D162497
This patch moves where we perform the final KillClearOnShutdown to occur
after we've shut down non-main threads, but before the main thread stops
accepting events. This should help ensure that unsuspecting events,
including those triggered from nsIThreadShutdownTask tasks, don't run
after KillClearOnShutdown has been run on background or main threads.
This KillClearOnShutdown was moved to occur before
nsThreadManager::Shutdown() in bug 1637890, as there were examples of
KillClearOnShutdown callbacks which needed to be able to dispatch
main-thread runnables. This change should not regress that use-case, as
we are still accepting new events on the main thread after the callback.
Non-main threads were already unreliable after this call as we already
block normal dispatches by setting gXPCOMThreadsShutdown, and new
threads cannot be started for the background thread pool.
Differential Revision: https://phabricator.services.mozilla.com/D144591
In future parts, TaskQueue will require extra initialization to be performed
which cannot happen in a constructor, as it takes references to the TaskQueue
object itself, which will require the introduction of a helper method. This
patch switches all callers of the TaskQueue constructor to use the new method.
Differential Revision: https://phabricator.services.mozilla.com/D142604
This patch moves where we perform the final KillClearOnShutdown to occur
after we've shut down non-main threads, but before the main thread stops
accepting events. This should help ensure that unsuspecting events,
including those triggered from nsIThreadShutdownTask tasks, don't run
after KillClearOnShutdown has been run on background or main threads.
This KillClearOnShutdown was moved to occur before
nsThreadManager::Shutdown() in bug 1637890, as there were examples of
KillClearOnShutdown callbacks which needed to be able to dispatch
main-thread runnables. This change should not regress that use-case, as
we are still accepting new events on the main thread after the callback.
Non-main threads were already unreliable after this call as we already
block normal dispatches by setting gXPCOMThreadsShutdown, and new
threads cannot be started for the background thread pool.
Differential Revision: https://phabricator.services.mozilla.com/D144591
We suspect to see the parent processes annotation on crash reports from child processes that result from IPC shutdown kills. In order to reduce the confusion we add a prefix for the process type.
Differential Revision: https://phabricator.services.mozilla.com/D131719
This patch:
- moves the AutoNestedEventLoopAnnotation into SpinEventLoopUntil.h
- introduces the motivated SpinEventLoopUntil
- maps remaining SpinEventLoopUntil instances to SpinEventLoopUntil with "Missing motivation."
- changes relevant uses in nsThread and nsThreadManager to the motivated SpinEventLoopUntil
- changes all uses with IgnoreAndContinue behavior to the motivated SpinEventLoopUntil
Differential Revision: https://phabricator.services.mozilla.com/D126714
Denis Palmeiro