Commit 8e4ff348 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Avoid running TLS destructors at background priority

This CL is adding a ScopedThreadPriority that save the current
thread priority and restore it when leaving the scope. This is
required since it is possible to run code outside of the Run(...)
scope with Background priority.

On Windows, this is causing shutdown hangs because the LoaderLock
can be held by a background thread while doing TLS destruction.


Without this fix, the DCHECK in the code run with Background priority
and fails:

[9888:28656:0617/210735.064:FATAL:thread_local_storage.cc(331)] Check failed: PlatformThread::GetCurrentThreadPriority() != ThreadPriority::BACKGROUND (0 vs. 0)
Backtrace:
        base::debug::CollectStackTrace [0x00007FF92445E7D2+18] (C:\src\chromium\src\base\debug\stack_trace_win.cc:284)
        base::debug::StackTrace::StackTrace [0x00007FF9243668F2+18] (C:\src\chromium\src\base\debug\stack_trace.cc:203)
        logging::LogMessage::~LogMessage [0x00007FF92437F094+148] (C:\src\chromium\src\base\logging.cc:605)
        logging::LogMessage::~LogMessage [0x00007FF92437FE40+16] (C:\src\chromium\src\base\logging.cc:598)
        base::internal::PlatformThreadLocalStorage::OnThreadExit [0x00007FF9244425FA+170] (C:\src\chromium\src\base\threading\thread_local_storage.cc:333)
        RtlActivateActivationContextUnsafeFast [0x00007FF96440B583+291]
        RtlActivateActivationContextUnsafeFast [0x00007FF96440B67F+543]
        LdrShutdownThread [0x00007FF964408B8C+348]
        RtlExitUserThread [0x00007FF9644436FE+62]
        BaseThreadInitThunk [0x00007FF96240403C+28]
        RtlUserThreadStart [0x00007FF964443691+33]

Bug: 1096203
Change-Id: Ibc011ac370dde4b6a872da6ac81938f52965a5a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2251199
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779932}
parent 9acc70c7
......@@ -16,6 +16,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/scoped_thread_priority.h"
#include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_restrictions.h"
#include "base/time/time_override.h"
......@@ -115,6 +116,13 @@ DWORD __stdcall ThreadFunc(void* params) {
PlatformThread::CurrentId());
}
// Ensure thread priority is at least NORMAL before initiating thread
// destruction. Thread destruction on Windows holds the LdrLock while
// performing TLS destruction which causes hangs if performed at background
// priority (priority inversion) (see: http://crbug.com/1096203).
if (PlatformThread::GetCurrentThreadPriority() < ThreadPriority::NORMAL)
PlatformThread::SetCurrentThreadPriority(ThreadPriority::NORMAL);
return 0;
}
......
......@@ -10,6 +10,7 @@
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/synchronization/lock.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h"
using base::internal::PlatformThreadLocalStorage;
......@@ -327,6 +328,10 @@ namespace internal {
#if defined(OS_WIN)
void PlatformThreadLocalStorage::OnThreadExit() {
// Don't execute TLS destructor at low priority. On Windows, this leads to
// LoaderLock priority inversions and shutdown hangs.
DCHECK_GE(PlatformThread::GetCurrentThreadPriority(), ThreadPriority::NORMAL);
PlatformThreadLocalStorage::TLSKey key =
base::subtle::NoBarrier_Load(&g_native_tls_key);
if (key == PlatformThreadLocalStorage::TLS_KEY_OUT_OF_INDEXES)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment