Investigate and Fix If Necessary Thread Destruction Callback Order |
||
Issue descriptionhttp://crbug.com/825218 discovered that during thread destruction, a third-party component that triggers Chrome related code can recreate the Chrome per-thread TLS vector. The sequence of events as follows is for Windows only: 1) Thread destruction starts. 2) PlatformThreadLocalStorage::OnThreadExit runs, destroying the per-thread TLS vector. 3) Windows nexts calls a third-party's (e.g. BoringSSL) thread exit handler. 4) Memory Infra's malloc+free hooks get called. 5) ThreadLocalStorage::Slot::Get recreates the per-thread TLS vector. 6) The third-party's exit handler completes. 7) The thread terminates and the per-thread TLS vector is leaked. An invariant of most OSes is that if a thread is executing code, the OS state associated with that thread remains available. Because Chrome performed a level of indirection for TLS but did not perform an equivalent indirection for the thread callback functions, the invariant is violated as it is possible to execute code without a backing per-thread TLS vector. This is less of an issue in POSIX as POSIX has destruction metadata per OS TLS slot. If an OS TLS slot is allocated during destruction, another pass will occur until all slots are freed. The fact that Chrome's TLS system itself performs similar looping logic as POSIX is an implicit acknowledgement of components needing TLS during thread destruction. The investigation for this bug is to see what is required to maintain the OS thread-state and execution invariant: the backing data for TLS should always be available while any code is running.
,
Mar 26 2018
FWIW, I don't believe we should do anything here. //base's TLS registrations do outlive everything in our codebase that uses TLS. The problem is only with hooks that result in re-entering chrome after it's done on that thread. Such hooks should support this reentrancy if they use TLS (and the aforementioned issue will add a DCHECK to endure this is the case). I'm against another hook just to be "absolute last" (and there's always the possibility of a module hooking our hook so it is "absolute last"...)
,
Mar 26 2018
http://crbug.com/825218 suggest that base is somehow getting outlived, which will require further investigation.
,
Mar 26 2018
My point is it's not outlived by something in our codebase, it's something in third_party. The third_party registration for thread exit is legitimate and independent, it's the memory profiling code that enables reentrancy by hooking Free() and hence it's responsible to protect itself against that.
,
Jan 11
Available, but no owner or component? Please find a component, as no one will ever find this without one. |
||
►
Sign in to add a comment |
||
Comment 1 by robliao@chromium.org
, Mar 26 2018