New issue
Advanced search Search tips

Issue 698303 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

WTF::ThreadSpecific can fail due to false positives from mayNotBeOnMainThread

Project Member Reported by jbroman@chromium.org, Mar 3 2017

Issue description

WTF::ThreadSpecific::m_mainThreadStorage exists to provide faster access on the main thread on platforms where we believe TLS to be slow (Win, Mac).

For this, it uses WTF::internal::mayNotBeOnMainThread, which has a contract that it may return true even when on the main thread (because it intentionally uses an underestimate of the main thread stack range).

Unfortunately, while it's okay to not _read_ the main-thread storage if we're not on the main thread, we must always write this storage because it could be read later.

Suppose the following access sequence occurs:
1. ThreadSpecific<T> is accessed on the main thread
  - mayNotBeMainThread returns true (e.g. because the stack happens to be large at the time)
  - a stack-local |offThreadPtr| is used instead of |m_mainThreadStorage|
  - a new T is allocated and stored using ThreadSpecific::set, but not mirrored in |m_mainThreadStorage|
2. The same ThreadSpecific<T> is accessed on the main thread
  - mayNotBeMainThread returns false (e.g. because we are back in the expected stack range)
  - the data is read from |m_mainThreadStorage|, which is found null
  - another new T is allocated and stored using ThreadSpecific::set
  - DCHECK fires in ThreadSpecific::set (if DCHECK is not enabled, we allocate a second T)

Given its contract, callers of mayNotBeMainThread should continue to work if it always returns true (albeit slower) -- this is not the case today.
 
Thanks for finding this bug... Your analysis is correct.

Forgot to update the BUG= line, but this is also fixed by https://codereview.chromium.org/2728453006.
Status: Fixed (was: Started)

Sign in to add a comment