Skip to content

Commit

Permalink
Revert "Use thread_local for WTF::IsMainThread."
Browse files Browse the repository at this point in the history
This reverts commit 7fa769e.

Bug: 927285
Change-Id: I003834a44cc27075be7071fca091869bb85e427f
Reviewed-on: https://chromium-review.googlesource.com/c/1448680
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628209}
  • Loading branch information
tguilbert-google authored and Commit Bot committed Feb 1, 2019
1 parent e85e660 commit 91dff48
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 33 deletions.
16 changes: 8 additions & 8 deletions styleguide/c++/c++11.html
Original file line number Diff line number Diff line change
Expand Up @@ -318,14 +318,6 @@ <h2 id="whitelist"><a name="core-whitelist"></a>C++11 Allowed Features</h2>
<td>Use only where it considerably improves readability. <a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/OQyYSfH9m2M">Discussion thread</a>. <a href="https://groups.google.com/a/chromium.org/forum/#!topic/cxx/Lkp0nubVd0Q">Another discussion thread</a></td>
</tr>

<tr>
<td>thread_local storage class</td>
<td><code>thread_local int foo = 1;</code></td>
<td>Puts variables into thread local storage.</td>
<td><a href="http://en.cppreference.com/w/cpp/language/storage_duration">Storage duration</a></td>
<td>Allowed only for POD data. (<a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/2msN8k3Xzgs">discussion</a>, <a href="https://groups.google.com/a/chromium.org/forum/#!topic/cxx/h7O5BdtWCZw">fork</a>). Use <a href="https://cs.chromium.org/chromium/src/base/threading/sequence_local_storage_slot.h">SequenceLocalStorageSlot</a> for sequence support, and <a href="https://cs.chromium.org/chromium/src/base/threading/thread_local.h">ThreadLocal</a>/<a href="https://cs.chromium.org/chromium/src/base/threading/thread_local_storage.h">ThreadLocalStorage</a> for other cases.</td>
</tr>

<tr>
<td>Type Aliases ("using" instead of "typedef")</td>
<td><code>using <i>new_alias</i> = <i>typename</i></code></td>
Expand Down Expand Up @@ -784,6 +776,14 @@ <h3 id="blacklist_banned"><a name="core-blacklist"></a>C++11 Banned Features</h3
<td>Banned in the <a href="https://google.github.io/styleguide/cppguide.html#Operator_Overloading">Google Style Guide</a>.</td>
</tr>

<tr>
<td>thread_local storage class</td>
<td><code>thread_local int foo = 1;</code></td>
<td>Puts variables into thread local storage.</td>
<td><a href="http://en.cppreference.com/w/cpp/language/storage_duration">Storage duration</a></td>
<td>Some surprising effects on Mac (<a href="https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/2msN8k3Xzgs">discussion</a>, <a href="https://groups.google.com/a/chromium.org/forum/#!topic/cxx/h7O5BdtWCZw">fork</a>). Use <a href="https://cs.chromium.org/chromium/src/base/threading/sequence_local_storage_slot.h">SequenceLocalStorageSlot</a> for sequence support, and <a href="https://cs.chromium.org/chromium/src/base/threading/thread_local.h">ThreadLocal</a>/<a href="https://cs.chromium.org/chromium/src/base/threading/thread_local_storage.h">ThreadLocalStorage</a> otherwise.</td>
</tr>

</tbody>
</table>

Expand Down
5 changes: 1 addition & 4 deletions third_party/blink/renderer/platform/wtf/thread_specific.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ inline void ThreadSpecific<T>::Destroy(void* ptr) {
// longer has a graceful shutdown sequence. Be careful to call this function
// (which can be re-entrant) while the pointer is still set, to avoid lazily
// allocating WTFThreadData after it is destroyed.
// This check cannot be performed using WTF::IsMainThread, as it uses TLS to
// store thread information, and we can't rely on the destruction order of
// TLS variables.
if (CurrentThread() == g_main_thread_identifier)
if (IsMainThread())
return;

// The memory was allocated via Partitions::FastZeroedMalloc, and then the
Expand Down
14 changes: 4 additions & 10 deletions third_party/blink/renderer/platform/wtf/wtf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,6 @@ bool g_initialized;
void (*g_call_on_main_thread_function)(MainThreadFunction, void*);
base::PlatformThreadId g_main_thread_identifier;

#if defined(COMPONENT_BUILD) && defined(OS_WIN)
static thread_local bool g_is_main_thread = false;
bool IsMainThread() {
return g_is_main_thread;
}
#else
thread_local bool g_is_main_thread = false;
#endif

namespace internal {

void CallOnMainThread(MainThreadFunction* function, void* context) {
Expand All @@ -66,13 +57,16 @@ void CallOnMainThread(MainThreadFunction* function, void* context) {

} // namespace internal

bool IsMainThread() {
return CurrentThread() == g_main_thread_identifier;
}

void Initialize(void (*call_on_main_thread_function)(MainThreadFunction,
void*)) {
// WTF, and Blink in general, cannot handle being re-initialized.
// Make that explicit here.
CHECK(!g_initialized);
g_initialized = true;
g_is_main_thread = true;
g_main_thread_identifier = CurrentThread();

WTFThreadData::Initialize();
Expand Down
11 changes: 0 additions & 11 deletions third_party/blink/renderer/platform/wtf/wtf.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_WTF_WTF_H_
#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_WTF_WTF_H_

#include "build/build_config.h"
#include "third_party/blink/renderer/platform/wtf/compiler.h"
#include "third_party/blink/renderer/platform/wtf/threading.h"
#include "third_party/blink/renderer/platform/wtf/wtf_export.h"
Expand All @@ -44,17 +43,7 @@ WTF_EXPORT extern base::PlatformThreadId g_main_thread_identifier;
// This function must be called exactly once from the main thread before using
// anything else in WTF.
WTF_EXPORT void Initialize(void (*)(MainThreadFunction, void*));

// thread_local variables can't be exported on Windows, so we use an extra
// function call on component builds.
#if defined(COMPONENT_BUILD) && defined(OS_WIN)
WTF_EXPORT bool IsMainThread();
#else
WTF_EXPORT extern thread_local bool g_is_main_thread;
inline bool IsMainThread() {
return g_is_main_thread;
}
#endif

namespace internal {
void CallOnMainThread(MainThreadFunction*, void* context);
Expand Down

0 comments on commit 91dff48

Please sign in to comment.