Title: [153499] trunk/Source/WebKit2
- Revision
- 153499
- Author
- [email protected]
- Date
- 2013-07-30 15:20:21 -0700 (Tue, 30 Jul 2013)
Log Message
Speculative fix for crash due to string access on multiple threads
https://bugs.webkit.org/show_bug.cgi?id=119279
<rdar://problem/14267833>
Reviewed by Darin Adler.
We can't use String::isolatedCopy for passing the local storage directory to the storage thread since
that returns a String that's copied and then destroyed after the call to bind returns, leaving a small window
where the refcount can be accessed simultaneously from two threads.
Work around this by passing a PassRefPtr<StringImpl> to bind instead; the act of copying the PassRefPtr will
clear out the original and so when the original is destroyed the underlying StringImpl pointer will be null.
* UIProcess/Storage/LocalStorageDatabaseTracker.cpp:
(WebKit::LocalStorageDatabaseTracker::setLocalStorageDirectory):
(WebKit::LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal):
* UIProcess/Storage/LocalStorageDatabaseTracker.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (153498 => 153499)
--- trunk/Source/WebKit2/ChangeLog 2013-07-30 22:17:20 UTC (rev 153498)
+++ trunk/Source/WebKit2/ChangeLog 2013-07-30 22:20:21 UTC (rev 153499)
@@ -1,3 +1,23 @@
+2013-07-30 Anders Carlsson <[email protected]>
+
+ Speculative fix for crash due to string access on multiple threads
+ https://bugs.webkit.org/show_bug.cgi?id=119279
+ <rdar://problem/14267833>
+
+ Reviewed by Darin Adler.
+
+ We can't use String::isolatedCopy for passing the local storage directory to the storage thread since
+ that returns a String that's copied and then destroyed after the call to bind returns, leaving a small window
+ where the refcount can be accessed simultaneously from two threads.
+
+ Work around this by passing a PassRefPtr<StringImpl> to bind instead; the act of copying the PassRefPtr will
+ clear out the original and so when the original is destroyed the underlying StringImpl pointer will be null.
+
+ * UIProcess/Storage/LocalStorageDatabaseTracker.cpp:
+ (WebKit::LocalStorageDatabaseTracker::setLocalStorageDirectory):
+ (WebKit::LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal):
+ * UIProcess/Storage/LocalStorageDatabaseTracker.h:
+
2013-07-30 Tim Horton <[email protected]>
Fix typo in enum name ("SelectionInSnaphot" -> "SelectionInSnapshot")
Modified: trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.cpp (153498 => 153499)
--- trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.cpp 2013-07-30 22:17:20 UTC (rev 153498)
+++ trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.cpp 2013-07-30 22:20:21 UTC (rev 153499)
@@ -52,7 +52,10 @@
void LocalStorageDatabaseTracker::setLocalStorageDirectory(const String& localStorageDirectory)
{
- m_queue->dispatch(bind(&LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal, this, localStorageDirectory.isolatedCopy()));
+ // FIXME: We should come up with a better idiom for safely copying strings across threads.
+ RefPtr<StringImpl> copiedLocalStorageDirectory = localStorageDirectory.impl() ? localStorageDirectory.impl()->isolatedCopy() : nullptr;
+
+ m_queue->dispatch(bind(&LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal, this, copiedLocalStorageDirectory.release()));
}
String LocalStorageDatabaseTracker::databasePath(SecurityOrigin* securityOrigin) const
@@ -129,7 +132,7 @@
return origins;
}
-void LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal(const String& localStorageDirectory)
+void LocalStorageDatabaseTracker::setLocalStorageDirectoryInternal(StringImpl* localStorageDirectory)
{
if (m_database.isOpen())
m_database.close();
Modified: trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.h (153498 => 153499)
--- trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.h 2013-07-30 22:17:20 UTC (rev 153498)
+++ trunk/Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.h 2013-07-30 22:20:21 UTC (rev 153499)
@@ -59,7 +59,7 @@
private:
explicit LocalStorageDatabaseTracker(PassRefPtr<WorkQueue>);
- void setLocalStorageDirectoryInternal(const String&);
+ void setLocalStorageDirectoryInternal(StringImpl*);
String databasePath(const String& filename) const;
String trackerDatabasePath() const;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
