Changeset 189526 in webkit

Timestamp:

Sep 8, 2015 6:38:32 PM (9 years ago)

Author:

Michael Catanzaro

Message:

Crash when WebCore::SQLiteFileSystem::openDatabase is called from multiple threads
​https://bugs.webkit.org/show_bug.cgi?id=143245

Reviewed by Darin Adler.

sqlite3_initialize is documented to be thread-safe, and to be called automatically by the
library when needed, so applications should never need to call it directly. The problem is,
it's not thread-safe: we have documented instances of GNOME Builder, Devhelp, Epiphany, and
cinnamon-screensaver crashing when sqlite3_initialize is called simultaneously in separate
threads (usually inside sqlite3_open). So call it manually, guarded using std::call_once, to
make sure that the library is fully initialized before the first call to sqlite3_open. It's
a good idea to do this regardless, because the documentation says it could be required in
a future release of SQLite. (Though the use of std::call_once should not be needed, and is
only used to attempt to work around the crashes.)

This is a workaround for an SQLite bug that might have been fixed upstream, but the SQLite
developers are not really confident in the thread-safety of this function, and have advised
that we carry the workaround. Seems like a good idea.

• platform/sql/SQLiteDatabase.cpp:

(WebCore::SQLiteDatabase::SQLiteDatabase):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/sql/SQLiteDatabase.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-09-08  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        Crash when WebCore::SQLiteFileSystem::openDatabase is called from multiple threads
+        https://bugs.webkit.org/show_bug.cgi?id=143245
+
+        Reviewed by Darin Adler.
+
+        sqlite3_initialize is documented to be thread-safe, and to be called automatically by the
+        library when needed, so applications should never need to call it directly. The problem is,
+        it's not thread-safe: we have documented instances of GNOME Builder, Devhelp, Epiphany, and
+        cinnamon-screensaver crashing when sqlite3_initialize is called simultaneously in separate
+        threads (usually inside sqlite3_open). So call it manually, guarded using std::call_once, to
+        make sure that the library is fully initialized before the first call to sqlite3_open. It's
+        a good idea to do this regardless, because the documentation says it could be required in
+        a future release of SQLite. (Though the use of std::call_once should not be needed, and is
+        only used to attempt to work around the crashes.)
+
+        This is a workaround for an SQLite bug that might have been fixed upstream, but the SQLite
+        developers are not really confident in the thread-safety of this function, and have advised
+        that we carry the workaround. Seems like a good idea.
+
+        * platform/sql/SQLiteDatabase.cpp:
+        (WebCore::SQLiteDatabase::SQLiteDatabase):
+
 2015-09-08  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp

@@ -32 +32 @@
 #include "SQLiteFileSystem.h"
 #include "SQLiteStatement.h"
+#include <mutex>
+#include <sqlite3.h>
 #include <thread>
 #include <wtf/Threading.h>
@@ -58 +60 @@
     , m_lastChangesCount(0)
 {
+    static std::once_flag flag;
+    std::call_once(flag, [] {
+        // It should be safe to call this outside of std::call_once, since it is documented to be
+        // completely threadsafe. But in the past it was not safe, and the SQLite developers still
+        // aren't confident that it really is, and we still support ancient versions of SQLite. So
+        // std::call_once is used to stay on the safe side. See bug #143245.
+        int ret = sqlite3_initialize();
+        if (ret != SQLITE_OK) {
+#if SQLITE_VERSION_NUMBER >= 3007015
+            WTFLogAlways("Failed to initialize SQLite: %s", sqlite3_errstr(ret));
+#else
+            WTFLogAlways("Failed to initialize SQLite");
+#endif
+            CRASH();
+        }
+    });
 }