1192194 - DOMStorageDBThead::mDBReady should be atomic

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Honza Bambas (:mayhemer)]

It's read on two threads and block database update and main thread direct read concurrency.

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8644916 [details] [diff] [review]
v1

It is not clear to me whether this is enough.
DOMStorageDBThread::ShutdownDatabase() sets the value back to false, but what
if we have just executed
if (mDBReady && mWALModeEnabled) { in DOMStorageDBThread::SyncPreload.
Should we protect the stuff using a monitor?

But r+ to this change.

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Olli Pettay [:smaug] (reviewing overload, please consider other reviewers) from comment #2)
> Comment on attachment 8644916 [details] [diff] [review]
> v1
> 
> It is not clear to me whether this is enough.
> DOMStorageDBThread::ShutdownDatabase() sets the value back to false, but what
> if we have just executed
> if (mDBReady && mWALModeEnabled) { in DOMStorageDBThread::SyncPreload.
> Should we protect the stuff using a monitor?
> 
> But r+ to this change.

nsresult
DOMStorageDBThread::ShutdownDatabase()
{
  // Has to be called on the worker thread.
  MOZ_ASSERT(!NS_IsMainThread());

  nsresult rv = mStatus;

  mDBReady = false;

  // Finalize the cached statements.
  mReaderStatements.FinalizeStatements();
  mWorkerStatements.FinalizeStatements();

  if (mReaderConnection) {
    // No need to sync access to mReaderConnection since the main thread
    // is right now joining this thread, unable to execute any events.

^^^^^

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3d0fe2a731d

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f3d0fe2a731d