To be more specific for (1).

On POSIX, we've observed the following sequence of events:

0) Thread starts destruction.
1) base::TLS destroys internal state.
2) ThreadSpecific begins clean-up, which invokes base::TLS, thus causing internal state to be created again
3) base::TLS destroys internal state [again].

This causes unnecessary work.

This would cause a leak on Windows, since thread destruction function hooks are single pass [rather than multi-pass on POSIX]. However, this currently doesn't cause a leak because the thread destruction hooks don't fire on Windows AFAICT.

Rewording of c#1, as the initial wording caused confusion.

To be more specific for (1).

On POSIX, we've observed the following sequence of events:

0) Thread starts destruction.
1) base::TLS destroys internal state.
2) ThreadSpecific begins clean-up, which invokes base::TLS, thus causing internal state to be created again
3) base::TLS destroys internal state [again].

This causes unnecessary work. Ideally, we wouldn't initialize and then destroy internal state in base::TLS.


On Windows, there are no thread-destruction TLS hooks. There are generic thread-destruction hooks that are invoked exactly once, in a non-deterministic ordering. See base/threading/thread_local_storage_win.cc.

Currently, ThreadSpecific does not implement any thread-destruction hooks on Windows. If it were to do so using generic mechanism described in thread_local_storage_win.cc, this would cause memory leaks, since ThreadSpecific::Destroy can cause base::TLS to reinitialize internal state.

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

commit e87cffa68c3d964c61909b2ea31603ac05cf6af2
Author: erikchen <erikchen@chromium.org>
Date: Fri Mar 30 18:15:40 2018

Update ThreadSpecific to use base::ThreadLocalStorage.

The current implementation has two problems:

1) It can call into base::TLS after the destructor of base::TLS has been
invoked. This is highly undesirable, as it will cause extra, unnecessary work on
POSIX, and causes memory leaks on Windows.

Technically this isn't an issue on Windows, but only because
ThreadSpecific::Destroy isn't called, which in itself is a bug that causes
memory leaks. See next point.

2) ThreadSpecific::Destroy isn't called on Windows, resulting in memory leaks.
See https://bugs.chromium.org/p/chromium/issues/detail?id=718110.

Bug: 825218,  826397 , 718110
Change-Id: I88f383621abb4c7fe904688a314afe06e8f1e5b0
Reviewed-on: https://chromium-review.googlesource.com/984522
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547209}
[modify] https://crrev.com/e87cffa68c3d964c61909b2ea31603ac05cf6af2/third_party/WebKit/Source/platform/wtf/BUILD.gn
[modify] https://crrev.com/e87cffa68c3d964c61909b2ea31603ac05cf6af2/third_party/WebKit/Source/platform/wtf/DEPS
[modify] https://crrev.com/e87cffa68c3d964c61909b2ea31603ac05cf6af2/third_party/WebKit/Source/platform/wtf/ThreadSpecific.h
[delete] https://crrev.com/34cee7038e43d6537a80b5f4aa311b3b783b13b2/third_party/WebKit/Source/platform/wtf/ThreadSpecificWin.cpp
[modify] https://crrev.com/e87cffa68c3d964c61909b2ea31603ac05cf6af2/third_party/WebKit/Source/platform/wtf/ThreadingPthreads.cpp
[modify] https://crrev.com/e87cffa68c3d964c61909b2ea31603ac05cf6af2/third_party/WebKit/Source/platform/wtf/ThreadingWin.cpp