Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(244)

Side by Side Diff: Source/WebCore/storage/AbstractDatabase.cpp

Issue 8152004: Merge 96554 - A little more WebSQLDatabase thread safety. (Closed) Base URL: http://svn.webkit.org/repository/webkit/branches/chromium/874/
Patch Set: Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (C) 2010 Google Inc. All rights reserved. 2 * Copyright (C) 2011 Google Inc. All rights reserved.
3 * 3 *
4 * Redistribution and use in source and binary forms, with or without 4 * Redistribution and use in source and binary forms, with or without
5 * modification, are permitted provided that the following conditions 5 * modification, are permitted provided that the following conditions
6 * are met: 6 * are met:
7 * 7 *
8 * 1. Redistributions of source code must retain the above copyright 8 * 1. Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * 2. Redistributions in binary form must reproduce the above copyright 10 * 2. Redistributions in binary form must reproduce the above copyright
11 * notice, this list of conditions and the following disclaimer in the 11 * notice, this list of conditions and the following disclaimer in the
12 * documentation and/or other materials provided with the distribution. 12 * documentation and/or other materials provided with the distribution.
(...skipping 27 matching lines...) Expand all
40 #include <wtf/HashMap.h> 40 #include <wtf/HashMap.h>
41 #include <wtf/HashSet.h> 41 #include <wtf/HashSet.h>
42 #include <wtf/PassRefPtr.h> 42 #include <wtf/PassRefPtr.h>
43 #include <wtf/RefPtr.h> 43 #include <wtf/RefPtr.h>
44 #include <wtf/StdLibExtras.h> 44 #include <wtf/StdLibExtras.h>
45 #include <wtf/text/CString.h> 45 #include <wtf/text/CString.h>
46 #include <wtf/text/StringHash.h> 46 #include <wtf/text/StringHash.h>
47 47
48 namespace WebCore { 48 namespace WebCore {
49 49
50 static const char versionKey[] = "WebKitDatabaseVersionKey";
51 static const char infoTableName[] = "__WebKitDatabaseInfoTable__";
52
50 static bool retrieveTextResultFromDatabase(SQLiteDatabase& db, const String& que ry, String& resultString) 53 static bool retrieveTextResultFromDatabase(SQLiteDatabase& db, const String& que ry, String& resultString)
51 { 54 {
52 SQLiteStatement statement(db, query); 55 SQLiteStatement statement(db, query);
53 int result = statement.prepare(); 56 int result = statement.prepare();
54 57
55 if (result != SQLResultOk) { 58 if (result != SQLResultOk) {
56 LOG_ERROR("Error (%i) preparing statement to read text result from datab ase (%s)", result, query.ascii().data()); 59 LOG_ERROR("Error (%i) preparing statement to read text result from datab ase (%s)", result, query.ascii().data());
57 return false; 60 return false;
58 } 61 }
59 62
(...skipping 28 matching lines...) Expand all
88 LOG_ERROR("Failed to step statement to set value in database (%s)", quer y.ascii().data()); 91 LOG_ERROR("Failed to step statement to set value in database (%s)", quer y.ascii().data());
89 return false; 92 return false;
90 } 93 }
91 94
92 return true; 95 return true;
93 } 96 }
94 97
95 // FIXME: move all guid-related functions to a DatabaseVersionTracker class. 98 // FIXME: move all guid-related functions to a DatabaseVersionTracker class.
96 static Mutex& guidMutex() 99 static Mutex& guidMutex()
97 { 100 {
98 // Note: We don't have to use AtomicallyInitializedStatic here because 101 AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
99 // this function is called once in the constructor on the main thread
100 // before any other threads that call this function are used.
101 DEFINE_STATIC_LOCAL(Mutex, mutex, ());
102 return mutex; 102 return mutex;
103 } 103 }
104 104
105 typedef HashMap<int, String> GuidVersionMap; 105 typedef HashMap<int, String> GuidVersionMap;
106 static GuidVersionMap& guidToVersionMap() 106 static GuidVersionMap& guidToVersionMap()
107 { 107 {
108 // Ensure the the mutex is locked.
109 ASSERT(!guidMutex().tryLock());
108 DEFINE_STATIC_LOCAL(GuidVersionMap, map, ()); 110 DEFINE_STATIC_LOCAL(GuidVersionMap, map, ());
109 return map; 111 return map;
110 } 112 }
111 113
112 // NOTE: Caller must lock guidMutex(). 114 // NOTE: Caller must lock guidMutex().
113 static inline void updateGuidVersionMap(int guid, String newVersion) 115 static inline void updateGuidVersionMap(int guid, String newVersion)
114 { 116 {
115 // Ensure the the mutex is locked. 117 // Ensure the the mutex is locked.
116 ASSERT(!guidMutex().tryLock()); 118 ASSERT(!guidMutex().tryLock());
117 119
118 // Note: It is not safe to put an empty string into the guidToVersionMap() m ap. 120 // Note: It is not safe to put an empty string into the guidToVersionMap() m ap.
119 // That's because the map is cross-thread, but empty strings are per-thread. 121 // That's because the map is cross-thread, but empty strings are per-thread.
120 // The copy() function makes a version of the string you can use on the curr ent 122 // The copy() function makes a version of the string you can use on the curr ent
121 // thread, but we need a string we can keep in a cross-thread data structure . 123 // thread, but we need a string we can keep in a cross-thread data structure .
122 // FIXME: This is a quite-awkward restriction to have to program with. 124 // FIXME: This is a quite-awkward restriction to have to program with.
123 125
124 // Map null string to empty string (see comment above). 126 // Map null string to empty string (see comment above).
125 guidToVersionMap().set(guid, newVersion.isEmpty() ? String() : newVersion.th readsafeCopy()); 127 guidToVersionMap().set(guid, newVersion.isEmpty() ? String() : newVersion.th readsafeCopy());
126 } 128 }
127 129
128 typedef HashMap<int, HashSet<AbstractDatabase*>*> GuidDatabaseMap; 130 typedef HashMap<int, HashSet<AbstractDatabase*>*> GuidDatabaseMap;
129 static GuidDatabaseMap& guidToDatabaseMap() 131 static GuidDatabaseMap& guidToDatabaseMap()
130 { 132 {
133 // Ensure the the mutex is locked.
134 ASSERT(!guidMutex().tryLock());
131 DEFINE_STATIC_LOCAL(GuidDatabaseMap, map, ()); 135 DEFINE_STATIC_LOCAL(GuidDatabaseMap, map, ());
132 return map; 136 return map;
133 } 137 }
134 138
135 static int guidForOriginAndName(const String& origin, const String& name) 139 static int guidForOriginAndName(const String& origin, const String& name)
136 { 140 {
141 // Ensure the the mutex is locked.
142 ASSERT(!guidMutex().tryLock());
143
137 String stringID = origin + "/" + name; 144 String stringID = origin + "/" + name;
138 145
139 // Note: We don't have to use AtomicallyInitializedStatic here because
140 // this function is called once in the constructor on the main thread
141 // before any other threads that call this function are used.
142 DEFINE_STATIC_LOCAL(Mutex, stringIdentifierMutex, ());
143 MutexLocker locker(stringIdentifierMutex);
144 typedef HashMap<String, int> IDGuidMap; 146 typedef HashMap<String, int> IDGuidMap;
145 DEFINE_STATIC_LOCAL(IDGuidMap, stringIdentifierToGUIDMap, ()); 147 DEFINE_STATIC_LOCAL(IDGuidMap, stringIdentifierToGUIDMap, ());
146 int guid = stringIdentifierToGUIDMap.get(stringID); 148 int guid = stringIdentifierToGUIDMap.get(stringID);
147 if (!guid) { 149 if (!guid) {
148 static int currentNewGUID = 1; 150 static int currentNewGUID = 1;
149 guid = currentNewGUID++; 151 guid = currentNewGUID++;
150 stringIdentifierToGUIDMap.set(stringID, guid); 152 stringIdentifierToGUIDMap.set(stringID, guid);
151 } 153 }
152 154
153 return guid; 155 return guid;
154 } 156 }
155 157
156 static bool isDatabaseAvailable = true; 158 static bool isDatabaseAvailable = true;
157 159
158 bool AbstractDatabase::isAvailable() 160 bool AbstractDatabase::isAvailable()
159 { 161 {
160 return isDatabaseAvailable; 162 return isDatabaseAvailable;
161 } 163 }
162 164
163 void AbstractDatabase::setIsAvailable(bool available) 165 void AbstractDatabase::setIsAvailable(bool available)
164 { 166 {
165 isDatabaseAvailable = available; 167 isDatabaseAvailable = available;
166 } 168 }
167 169
168 // static 170 // static
169 const String& AbstractDatabase::databaseInfoTableName() 171 const char* AbstractDatabase::databaseInfoTableName()
170 { 172 {
171 DEFINE_STATIC_LOCAL(String, name, ("__WebKitDatabaseInfoTable__")); 173 return infoTableName;
172 return name;
173 } 174 }
174 175
175 AbstractDatabase::AbstractDatabase(ScriptExecutionContext* context, const String & name, const String& expectedVersion, 176 AbstractDatabase::AbstractDatabase(ScriptExecutionContext* context, const String & name, const String& expectedVersion,
176 const String& displayName, unsigned long esti matedSize) 177 const String& displayName, unsigned long esti matedSize)
177 : m_scriptExecutionContext(context) 178 : m_scriptExecutionContext(context)
178 , m_name(name.crossThreadString()) 179 , m_name(name.crossThreadString())
179 , m_expectedVersion(expectedVersion.crossThreadString()) 180 , m_expectedVersion(expectedVersion.crossThreadString())
180 , m_displayName(displayName.crossThreadString()) 181 , m_displayName(displayName.crossThreadString())
181 , m_estimatedSize(estimatedSize) 182 , m_estimatedSize(estimatedSize)
182 , m_guid(0) 183 , m_guid(0)
183 , m_opened(false) 184 , m_opened(false)
184 , m_new(false) 185 , m_new(false)
185 { 186 {
186 ASSERT(context->isContextThread()); 187 ASSERT(context->isContextThread());
187 m_contextThreadSecurityOrigin = m_scriptExecutionContext->securityOrigin(); 188 m_contextThreadSecurityOrigin = m_scriptExecutionContext->securityOrigin();
188 189
189 m_databaseAuthorizer = DatabaseAuthorizer::create(databaseInfoTableName()); 190 m_databaseAuthorizer = DatabaseAuthorizer::create(infoTableName);
190 191
191 if (m_name.isNull()) 192 if (m_name.isNull())
192 m_name = ""; 193 m_name = "";
193 194
194 m_guid = guidForOriginAndName(securityOrigin()->toString(), name);
195 { 195 {
196 MutexLocker locker(guidMutex()); 196 MutexLocker locker(guidMutex());
197 197 m_guid = guidForOriginAndName(securityOrigin()->toString(), name);
198 HashSet<AbstractDatabase*>* hashSet = guidToDatabaseMap().get(m_guid); 198 HashSet<AbstractDatabase*>* hashSet = guidToDatabaseMap().get(m_guid);
199 if (!hashSet) { 199 if (!hashSet) {
200 hashSet = new HashSet<AbstractDatabase*>; 200 hashSet = new HashSet<AbstractDatabase*>;
201 guidToDatabaseMap().set(m_guid, hashSet); 201 guidToDatabaseMap().set(m_guid, hashSet);
202 } 202 }
203 203
204 hashSet->add(this); 204 hashSet->add(this);
205 } 205 }
206 206
207 m_filename = DatabaseTracker::tracker().fullPathForDatabase(securityOrigin() , m_name); 207 m_filename = DatabaseTracker::tracker().fullPathForDatabase(securityOrigin() , m_name);
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
256 256
257 m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime); 257 m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
258 258
259 String currentVersion; 259 String currentVersion;
260 { 260 {
261 MutexLocker locker(guidMutex()); 261 MutexLocker locker(guidMutex());
262 262
263 GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid); 263 GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
264 if (entry != guidToVersionMap().end()) { 264 if (entry != guidToVersionMap().end()) {
265 // Map null string to empty string (see updateGuidVersionMap()). 265 // Map null string to empty string (see updateGuidVersionMap()).
266 currentVersion = entry->second.isNull() ? String("") : entry->second ; 266 currentVersion = entry->second.isNull() ? String("") : entry->second .threadsafeCopy();
267 LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data()); 267 LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
268 268
269 #if PLATFORM(CHROMIUM) 269 #if PLATFORM(CHROMIUM)
270 // Note: In multi-process browsers the cached value may be inaccurat e, but 270 // Note: In multi-process browsers the cached value may be inaccurat e, but
271 // we cannot read the actual version from the database without poten tially 271 // we cannot read the actual version from the database without poten tially
272 // inducing a form of deadlock, a busytimeout error when trying to 272 // inducing a form of deadlock, a busytimeout error when trying to
273 // access the database. So we'll use the cached value if we're able to read 273 // access the database. So we'll use the cached value if we're unabl e to read
274 // the value without waiting, and otherwise use the cached value (wh ich may be off). 274 // the value from the database file without waiting.
275 // FIXME: Add an async openDatabase method to the DatabaseAPI. 275 // FIXME: Add an async openDatabase method to the DatabaseAPI.
276 const int noSqliteBusyWaitTime = 0; 276 const int noSqliteBusyWaitTime = 0;
277 m_sqliteDatabase.setBusyTimeout(noSqliteBusyWaitTime); 277 m_sqliteDatabase.setBusyTimeout(noSqliteBusyWaitTime);
278 String versionFromDatabase; 278 String versionFromDatabase;
279 if (getVersionFromDatabase(versionFromDatabase, false)) { 279 if (getVersionFromDatabase(versionFromDatabase, false)) {
280 currentVersion = versionFromDatabase; 280 currentVersion = versionFromDatabase;
281 updateGuidVersionMap(m_guid, currentVersion); 281 updateGuidVersionMap(m_guid, currentVersion);
282 } 282 }
283 m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime); 283 m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
284 #endif 284 #endif
285 } else { 285 } else {
286 LOG(StorageAPI, "No cached version for guid %i", m_guid); 286 LOG(StorageAPI, "No cached version for guid %i", m_guid);
287 287
288 SQLiteTransaction transaction(m_sqliteDatabase); 288 SQLiteTransaction transaction(m_sqliteDatabase);
289 transaction.begin(); 289 transaction.begin();
290 if (!transaction.inProgress()) { 290 if (!transaction.inProgress()) {
291 LOG_ERROR("Unable to begin transaction while opening %s", databa seDebugName().ascii().data()); 291 LOG_ERROR("Unable to begin transaction while opening %s", databa seDebugName().ascii().data());
292 ec = INVALID_STATE_ERR; 292 ec = INVALID_STATE_ERR;
293 m_sqliteDatabase.close(); 293 m_sqliteDatabase.close();
294 return false; 294 return false;
295 } 295 }
296 296
297 if (!m_sqliteDatabase.tableExists(databaseInfoTableName())) { 297 String tableName(infoTableName);
298 if (!m_sqliteDatabase.tableExists(tableName)) {
298 m_new = true; 299 m_new = true;
299 300
300 if (!m_sqliteDatabase.executeCommand("CREATE TABLE " + databaseI nfoTableName() + " (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLAC E,value TEXT NOT NULL ON CONFLICT FAIL);")) { 301 if (!m_sqliteDatabase.executeCommand("CREATE TABLE " + tableName + " (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT N OT NULL ON CONFLICT FAIL);")) {
301 LOG_ERROR("Unable to create table %s in database %s", databa seInfoTableName().ascii().data(), databaseDebugName().ascii().data()); 302 LOG_ERROR("Unable to create table %s in database %s", infoTa bleName, databaseDebugName().ascii().data());
302 ec = INVALID_STATE_ERR; 303 ec = INVALID_STATE_ERR;
303 transaction.rollback(); 304 transaction.rollback();
304 m_sqliteDatabase.close(); 305 m_sqliteDatabase.close();
305 return false; 306 return false;
306 } 307 }
307 } else if (!getVersionFromDatabase(currentVersion, false)) { 308 } else if (!getVersionFromDatabase(currentVersion, false)) {
308 LOG_ERROR("Failed to get current version from database %s", data baseDebugName().ascii().data()); 309 LOG_ERROR("Failed to get current version from database %s", data baseDebugName().ascii().data());
309 ec = INVALID_STATE_ERR; 310 ec = INVALID_STATE_ERR;
310 transaction.rollback(); 311 transaction.rollback();
311 m_sqliteDatabase.close(); 312 m_sqliteDatabase.close();
(...skipping 70 matching lines...) Expand 10 before | Expand all | Expand 10 after
382 { 383 {
383 return m_estimatedSize; 384 return m_estimatedSize;
384 } 385 }
385 386
386 String AbstractDatabase::fileName() const 387 String AbstractDatabase::fileName() const
387 { 388 {
388 // Return a deep copy for ref counting thread safety 389 // Return a deep copy for ref counting thread safety
389 return m_filename.threadsafeCopy(); 390 return m_filename.threadsafeCopy();
390 } 391 }
391 392
392 // static
393 const String& AbstractDatabase::databaseVersionKey()
394 {
395 DEFINE_STATIC_LOCAL(String, key, ("WebKitDatabaseVersionKey"));
396 return key;
397 }
398
399 bool AbstractDatabase::getVersionFromDatabase(String& version, bool shouldCacheV ersion) 393 bool AbstractDatabase::getVersionFromDatabase(String& version, bool shouldCacheV ersion)
400 { 394 {
401 DEFINE_STATIC_LOCAL(String, getVersionQuery, ("SELECT value FROM " + databas eInfoTableName() + " WHERE key = '" + databaseVersionKey() + "';")); 395 String query(String("SELECT value FROM ") + infoTableName + " WHERE key = ' " + versionKey + "';");
402 396
403 m_databaseAuthorizer->disable(); 397 m_databaseAuthorizer->disable();
404 398
405 bool result = retrieveTextResultFromDatabase(m_sqliteDatabase, getVersionQue ry.threadsafeCopy(), version); 399 bool result = retrieveTextResultFromDatabase(m_sqliteDatabase, query, versio n);
406 if (result) { 400 if (result) {
407 if (shouldCacheVersion) 401 if (shouldCacheVersion)
408 setCachedVersion(version); 402 setCachedVersion(version);
409 } else 403 } else
410 LOG_ERROR("Failed to retrieve version from database %s", databaseDebugNa me().ascii().data()); 404 LOG_ERROR("Failed to retrieve version from database %s", databaseDebugNa me().ascii().data());
411 405
412 m_databaseAuthorizer->enable(); 406 m_databaseAuthorizer->enable();
413 407
414 return result; 408 return result;
415 } 409 }
416 410
417 bool AbstractDatabase::setVersionInDatabase(const String& version, bool shouldCa cheVersion) 411 bool AbstractDatabase::setVersionInDatabase(const String& version, bool shouldCa cheVersion)
418 { 412 {
419 // The INSERT will replace an existing entry for the database with the new v ersion number, due to the UNIQUE ON CONFLICT REPLACE 413 // The INSERT will replace an existing entry for the database with the new v ersion number, due to the UNIQUE ON CONFLICT REPLACE
420 // clause in the CREATE statement (see Database::performOpenAndVerify()). 414 // clause in the CREATE statement (see Database::performOpenAndVerify()).
421 DEFINE_STATIC_LOCAL(String, setVersionQuery, ("INSERT INTO " + databaseInfoT ableName() + " (key, value) VALUES ('" + databaseVersionKey() + "', ?);")); 415 String query(String("INSERT INTO ") + infoTableName + " (key, value) VALUES ('" + versionKey + "', ?);");
422 416
423 m_databaseAuthorizer->disable(); 417 m_databaseAuthorizer->disable();
424 418
425 bool result = setTextValueInDatabase(m_sqliteDatabase, setVersionQuery.threa dsafeCopy(), version); 419 bool result = setTextValueInDatabase(m_sqliteDatabase, query, version);
426 if (result) { 420 if (result) {
427 if (shouldCacheVersion) 421 if (shouldCacheVersion)
428 setCachedVersion(version); 422 setCachedVersion(version);
429 } else 423 } else
430 LOG_ERROR("Failed to set version %s in database (%s)", version.ascii().d ata(), setVersionQuery.ascii().data()); 424 LOG_ERROR("Failed to set version %s in database (%s)", version.ascii().d ata(), query.ascii().data());
431 425
432 m_databaseAuthorizer->enable(); 426 m_databaseAuthorizer->enable();
433 427
434 return result; 428 return result;
435 } 429 }
436 430
437 void AbstractDatabase::setExpectedVersion(const String& version) 431 void AbstractDatabase::setExpectedVersion(const String& version)
438 { 432 {
439 m_expectedVersion = version.threadsafeCopy(); 433 m_expectedVersion = version.threadsafeCopy();
440 } 434 }
(...skipping 98 matching lines...) Expand 10 before | Expand all | Expand 10 after
539 533
540 bool AbstractDatabase::isInterrupted() 534 bool AbstractDatabase::isInterrupted()
541 { 535 {
542 MutexLocker locker(m_sqliteDatabase.databaseMutex()); 536 MutexLocker locker(m_sqliteDatabase.databaseMutex());
543 return m_sqliteDatabase.isInterrupted(); 537 return m_sqliteDatabase.isInterrupted();
544 } 538 }
545 539
546 } // namespace WebCore 540 } // namespace WebCore
547 541
548 #endif // ENABLE(DATABASE) 542 #endif // ENABLE(DATABASE)
OLDNEW
« no previous file with comments | « Source/WebCore/storage/AbstractDatabase.h ('k') | Source/WebCore/storage/chromium/DatabaseTrackerChromium.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698