Skip to content

Commit

Permalink
Fix for sync filesystem app crash.
Browse files Browse the repository at this point in the history
Previously removing the SafePointScope had a risk of producing a dead lock if the following scenario happens:

1) The main thread starts a GC and tries to stop all other threads.
2) The worker thread is waiting on a signal without entering the SafePointScope. The main thread cannot stop the worker thread forever and thus cannot make progress. Dead lock.

However, (to avoid this kind of dead lock) code has been added to timeout the GC if the GCing thread fails at stopping other threads within 100 ms. So not entering SafePointScope will just cause the timeout -- it won't cause a dead lock.

This fixes a hard-to-reproduce timing bug that's been a big concern of one of our partners.

BUG=592124,608603
TEST=manual

Review-Url: https://codereview.chromium.org/2009093002
Cr-Commit-Position: refs/heads/master@{#399265}
(cherry picked from commit f466a07)

Review URL: https://codereview.chromium.org/2065823003 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#353}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
achuith committed Jun 14, 2016
1 parent a586a4f commit d023da3
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions content/child/fileapi/webfilesystem_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "third_party/WebKit/public/platform/WebFileSystemCallbacks.h"
#include "third_party/WebKit/public/platform/WebString.h"
#include "third_party/WebKit/public/platform/WebURL.h"
#include "third_party/WebKit/public/web/WebHeap.h"
#include "url/gurl.h"

using base::MakeTuple;
Expand All @@ -52,10 +51,7 @@ class WebFileSystemImpl::WaitableCallbackResults
}

void WaitAndRun() {
{
blink::WebHeap::SafePointScope safe_point;
results_available_event_.Wait();
}
results_available_event_.Wait();
Run();
}

Expand Down

0 comments on commit d023da3

Please sign in to comment.