Diff
Modified: trunk/Source/WTF/wtf/ObjectIdentifier.h (229615 => 229616)
--- trunk/Source/WTF/wtf/ObjectIdentifier.h 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WTF/wtf/ObjectIdentifier.h 2018-03-14 22:52:09 UTC (rev 229616)
@@ -66,7 +66,8 @@
}
uint64_t toUInt64() const { return m_identifier; }
-
+ explicit operator bool() const { return m_identifier; }
+
String loggingString() const
{
return String::number(m_identifier);
Modified: trunk/Source/WebCore/ChangeLog (229615 => 229616)
--- trunk/Source/WebCore/ChangeLog 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/ChangeLog 2018-03-14 22:52:09 UTC (rev 229616)
@@ -1,3 +1,48 @@
+2018-03-14 Youenn Fablet <[email protected]>
+
+ imported/w3c/web-platform-tests/WebCryptoAPI/wrapKey_unwrapKey/wrapKey_unwrapKey.worker.html is crashing
+ https://bugs.webkit.org/show_bug.cgi?id=183602
+
+ Reviewed by Chris Dumez.
+
+ Introduce a map of ScriptExecutionContext that is read/write protected using a Lock.
+ This allows introducing postTaskTo taking a ScriptExecutionContext identifier and callable from any thread.
+ Use that method in Crypto instead of refing/unrefing the context.
+ Lock only happens if context does some postTask activity. This is governed by calling or not the new contextIdentifier() getter.
+
+ Covered by crypto tests no longer failing m_workerGlobalScope->hasOneRef() assertion.
+
+ * crypto/CryptoAlgorithm.cpp:
+ (WebCore::dispatchAlgorithmOperation):
+ * crypto/algorithms/CryptoAlgorithmECDH.cpp:
+ (WebCore::CryptoAlgorithmECDH::deriveBits):
+ * crypto/algorithms/CryptoAlgorithmSHA1.cpp:
+ (WebCore::CryptoAlgorithmSHA1::digest):
+ * crypto/algorithms/CryptoAlgorithmSHA224.cpp:
+ (WebCore::CryptoAlgorithmSHA224::digest):
+ * crypto/algorithms/CryptoAlgorithmSHA256.cpp:
+ (WebCore::CryptoAlgorithmSHA256::digest):
+ * crypto/algorithms/CryptoAlgorithmSHA384.cpp:
+ (WebCore::CryptoAlgorithmSHA384::digest):
+ * crypto/algorithms/CryptoAlgorithmSHA512.cpp:
+ (WebCore::CryptoAlgorithmSHA512::digest):
+ * crypto/gcrypt/CryptoKeyRSAGCrypt.cpp:
+ (WebCore::CryptoKeyRSA::generatePair):
+ * crypto/mac/CryptoKeyRSAMac.cpp:
+ (WebCore::CryptoKeyRSA::generatePair):
+ * dom/Document.cpp:
+ (WebCore::Document::~Document):
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::allScriptExecutionContextsMapLock):
+ (WebCore::ScriptExecutionContext::ScriptExecutionContext):
+ (WebCore::ScriptExecutionContext::removeFromContextsMap):
+ (WebCore::ScriptExecutionContext::checkConsistency const):
+ (WebCore::ScriptExecutionContext::postTaskTo):
+ * dom/ScriptExecutionContext.h:
+ (WebCore::ScriptExecutionContext::contextIdentifier const):
+ * workers/WorkerGlobalScope.cpp:
+ (WebCore::WorkerGlobalScope::~WorkerGlobalScope):
+
2018-03-14 Chris Dumez <[email protected]>
Reduce use of SWServerToContextConnection::globalServerToContextConnection()
Modified: trunk/Source/WebCore/crypto/CryptoAlgorithm.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/CryptoAlgorithm.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/CryptoAlgorithm.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -95,26 +95,17 @@
template<typename ResultCallbackType, typename OperationType>
static void dispatchAlgorithmOperation(WorkQueue& workQueue, ScriptExecutionContext& context, ResultCallbackType&& callback, CryptoAlgorithm::ExceptionCallback&& exceptionCallback, OperationType&& operation)
{
- context.ref();
+ // There is no guarantee that callback and exceptionCallback will be destroyed in the thread in which they were created.
workQueue.dispatch(
- [operation = WTFMove(operation), callback = WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback), &context]() mutable {
+ [operation = WTFMove(operation), callback = WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback), contextIdentifier = context.contextIdentifier()]() mutable {
auto result = operation();
- if (result.hasException()) {
- // We should only dereference callbacks after being back to the Document/Worker threads.
- context.postTask(
- [ec = result.releaseException().code(), callback = WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback)](ScriptExecutionContext& context) {
- exceptionCallback(ec);
- context.deref();
- });
- return;
- }
-
- // We should only dereference callbacks after being back to the Document/Worker threads.
- context.postTask(
- [result = result.releaseReturnValue(), callback = WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback)](ScriptExecutionContext& context) {
- callback(result);
- context.deref();
- });
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [result = crossThreadCopy(result), callback = WTFMove(callback), exceptionCallback = WTFMove(exceptionCallback)](auto&) mutable {
+ if (result.hasException()) {
+ exceptionCallback(result.releaseException().code());
+ return;
+ }
+ callback(result.releaseReturnValue());
+ });
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -116,15 +116,12 @@
// This is a special case that can't use dispatchOperation() because it bundles
// the result validation and callback dispatch into unifiedCallback.
- context.ref();
workQueue.dispatch(
- [baseKey = WTFMove(baseKey), publicKey = ecParameters.publicKey.releaseNonNull(), length, unifiedCallback = WTFMove(unifiedCallback), &context]() mutable {
+ [baseKey = WTFMove(baseKey), publicKey = ecParameters.publicKey.releaseNonNull(), length, unifiedCallback = WTFMove(unifiedCallback), contextIdentifier = context.contextIdentifier()]() mutable {
auto derivedKey = platformDeriveBits(downcast<CryptoKeyEC>(baseKey.get()), downcast<CryptoKeyEC>(publicKey.get()));
- context.postTask(
- [derivedKey = WTFMove(derivedKey), length, unifiedCallback = WTFMove(unifiedCallback)](ScriptExecutionContext& context) mutable {
- unifiedCallback(WTFMove(derivedKey), length);
- context.deref();
- });
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [derivedKey = WTFMove(derivedKey), length, unifiedCallback = WTFMove(unifiedCallback)](auto&) mutable {
+ unifiedCallback(WTFMove(derivedKey), length);
+ });
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA1.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA1.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA1.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -51,13 +51,11 @@
return;
}
- context.ref();
- workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), &context]() mutable {
+ workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), contextIdentifier = context.contextIdentifier()]() mutable {
digest->addBytes(message.data(), message.size());
auto result = digest->computeHash();
- context.postTask([callback = WTFMove(callback), result = WTFMove(result)](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [callback = WTFMove(callback), result = WTFMove(result)](auto&) {
callback(result);
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA224.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA224.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA224.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -51,13 +51,11 @@
return;
}
- context.ref();
- workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), &context]() mutable {
+ workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), contextIdentifier = context.contextIdentifier()]() mutable {
digest->addBytes(message.data(), message.size());
auto result = digest->computeHash();
- context.postTask([callback = WTFMove(callback), result = WTFMove(result)](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [callback = WTFMove(callback), result = WTFMove(result)](auto&) {
callback(result);
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA256.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA256.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA256.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -51,13 +51,11 @@
return;
}
- context.ref();
- workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), &context]() mutable {
+ workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), contextIdentifier = context.contextIdentifier()]() mutable {
digest->addBytes(message.data(), message.size());
auto result = digest->computeHash();
- context.postTask([callback = WTFMove(callback), result = WTFMove(result)](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [callback = WTFMove(callback), result = WTFMove(result)](auto&) {
callback(result);
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA384.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA384.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA384.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -51,13 +51,11 @@
return;
}
- context.ref();
- workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), &context]() mutable {
+ workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), contextIdentifier = context.contextIdentifier()]() mutable {
digest->addBytes(message.data(), message.size());
auto result = digest->computeHash();
- context.postTask([callback = WTFMove(callback), result = WTFMove(result)](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [callback = WTFMove(callback), result = WTFMove(result)](auto&) {
callback(result);
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA512.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA512.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmSHA512.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -51,13 +51,11 @@
return;
}
- context.ref();
- workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), &context]() mutable {
+ workQueue.dispatch([digest = WTFMove(digest), message = WTFMove(message), callback = WTFMove(callback), contextIdentifier = context.contextIdentifier()]() mutable {
digest->addBytes(message.data(), message.size());
auto result = digest->computeHash();
- context.postTask([callback = WTFMove(callback), result = WTFMove(result)](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [callback = WTFMove(callback), result = WTFMove(result)](auto&) {
callback(result);
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -231,14 +231,12 @@
return;
}
- context->ref();
context->postTask(
- [algorithm, hash, hasHash, extractable, usage, publicKeySexp = publicKeySexp.release(), privateKeySexp = privateKeySexp.release(), callback = WTFMove(callback)](ScriptExecutionContext& context) mutable {
+ [algorithm, hash, hasHash, extractable, usage, publicKeySexp = publicKeySexp.release(), privateKeySexp = privateKeySexp.release(), callback = WTFMove(callback)](auto&) mutable {
auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, publicKeySexp, true, usage);
auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, privateKeySexp, extractable, usage);
callback(CryptoKeyPair { WTFMove(publicKey), WTFMove(privateKey) });
- context.deref();
});
}
Modified: trunk/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp (229615 => 229616)
--- trunk/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -293,24 +293,24 @@
// We only use the callback functions when back on the main/worker thread, but captured variables are copied on a secondary thread too.
KeyPairCallback* localCallback = new KeyPairCallback(WTFMove(callback));
VoidCallback* localFailureCallback = new VoidCallback(WTFMove(failureCallback));
- context->ref();
+ auto contextIdentifier = context->contextIdentifier();
+ // FIXME: There is a risk that localCallback and localFailureCallback are never freed.
+ // Fix this by using unique pointers and move them from one lambda to the other.
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
- ASSERT(context);
CCRSACryptorRef ccPublicKey;
CCRSACryptorRef ccPrivateKey;
CCCryptorStatus status = CCRSACryptorGeneratePair(modulusLength, e, &ccPublicKey, &ccPrivateKey);
if (status) {
WTFLogAlways("Could not generate a key pair, status %d", status);
- context->postTask([localCallback, localFailureCallback](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [localCallback, localFailureCallback](auto&) {
(*localFailureCallback)();
delete localCallback;
delete localFailureCallback;
- context.deref();
});
return;
}
- context->postTask([algorithm, hash, hasHash, extractable, usage, localCallback, localFailureCallback, ccPublicKey, ccPrivateKey](ScriptExecutionContext& context) {
+ ScriptExecutionContext::postTaskTo(contextIdentifier, [algorithm, hash, hasHash, extractable, usage, localCallback, localFailureCallback, ccPublicKey, ccPrivateKey](auto&) {
auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, ccPublicKey, true, usage);
auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, ccPrivateKey, extractable, usage);
@@ -318,7 +318,6 @@
delete localCallback;
delete localFailureCallback;
- context.deref();
});
});
}
Modified: trunk/Source/WebCore/dom/Document.cpp (229615 => 229616)
--- trunk/Source/WebCore/dom/Document.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -568,8 +568,10 @@
Document::~Document()
{
- bool wasRemoved = allDocumentsMap().remove(m_identifier);
- ASSERT_UNUSED(wasRemoved, wasRemoved);
+ ASSERT(allDocumentsMap().contains(m_identifier));
+ allDocumentsMap().remove(m_identifier);
+ // We need to remove from the contexts map very early in the destructor so that calling postTask() on this Document from another thread is safe.
+ removeFromContextsMap();
ASSERT(!renderView());
ASSERT(m_pageCacheState != InPageCache);
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (229615 => 229616)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -65,6 +65,19 @@
namespace WebCore {
using namespace Inspector;
+static Lock& allScriptExecutionContextsMapLock()
+{
+ static NeverDestroyed<Lock> lock;
+ return lock;
+}
+
+static HashMap<ScriptExecutionContextIdentifier, ScriptExecutionContext*>& allScriptExecutionContextsMap()
+{
+ static NeverDestroyed<HashMap<ScriptExecutionContextIdentifier, ScriptExecutionContext*>> contexts;
+ ASSERT(allScriptExecutionContextsMapLock().isLocked());
+ return contexts;
+}
+
struct ScriptExecutionContext::PendingException {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -87,6 +100,29 @@
{
}
+ScriptExecutionContextIdentifier ScriptExecutionContext::contextIdentifier() const
+{
+ ASSERT(isContextThread());
+ if (!m_contextIdentifier) {
+ Locker<Lock> locker(allScriptExecutionContextsMapLock());
+
+ m_contextIdentifier = generateObjectIdentifier<ScriptExecutionContextIdentifierType>();
+
+ ASSERT(!allScriptExecutionContextsMap().contains(m_contextIdentifier));
+ allScriptExecutionContextsMap().add(m_contextIdentifier, const_cast<ScriptExecutionContext*>(this));
+ }
+ return m_contextIdentifier;
+}
+
+void ScriptExecutionContext::removeFromContextsMap()
+{
+ if (m_contextIdentifier) {
+ Locker<Lock> locker(allScriptExecutionContextsMapLock());
+ ASSERT(allScriptExecutionContextsMap().contains(m_contextIdentifier));
+ allScriptExecutionContextsMap().remove(m_contextIdentifier);
+ }
+}
+
#if ASSERT_DISABLED
inline void ScriptExecutionContext::checkConsistency() const
@@ -116,6 +152,12 @@
checkConsistency();
#if !ASSERT_DISABLED
+ if (m_contextIdentifier) {
+ Locker<Lock> locker(allScriptExecutionContextsMapLock());
+ ASSERT_WITH_MESSAGE(!allScriptExecutionContextsMap().contains(m_contextIdentifier),
+ "A ScriptExecutionContext subclass instance implementing postTask should have already removed itself from the map");
+ }
+
m_inScriptExecutionContextDestructor = true;
#endif
@@ -580,6 +622,19 @@
});
return wasPosted;
}
+
#endif
+bool ScriptExecutionContext::postTaskTo(ScriptExecutionContextIdentifier identifier, Task&& task)
+{
+ Locker<Lock> locker(allScriptExecutionContextsMapLock());
+ auto* context = allScriptExecutionContextsMap().get(identifier);
+
+ if (!context)
+ return false;
+
+ context->postTask(WTFMove(task));
+ return true;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (229615 => 229616)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.h 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h 2018-03-14 22:52:09 UTC (rev 229616)
@@ -36,6 +36,7 @@
#include <wtf/CrossThreadTask.h>
#include <wtf/Function.h>
#include <wtf/HashSet.h>
+#include <wtf/ObjectIdentifier.h>
#include <wtf/text/WTFString.h>
namespace JSC {
@@ -78,6 +79,9 @@
class IDBConnectionProxy;
}
+enum ScriptExecutionContextIdentifierType { };
+using ScriptExecutionContextIdentifier = ObjectIdentifier<ScriptExecutionContextIdentifierType>;
+
class ScriptExecutionContext : public SecurityContext {
public:
ScriptExecutionContext();
@@ -253,7 +257,10 @@
WEBCORE_EXPORT static bool postTaskTo(const DocumentOrWorkerIdentifier&, WTF::Function<void(ScriptExecutionContext&)>&&);
#endif
+ WEBCORE_EXPORT static bool postTaskTo(ScriptExecutionContextIdentifier, Task&&);
+ ScriptExecutionContextIdentifier contextIdentifier() const;
+
protected:
class AddConsoleMessageTask : public Task {
public:
@@ -275,6 +282,7 @@
ActiveDOMObject::ReasonForSuspension reasonForSuspendingActiveDOMObjects() const { return m_reasonForSuspendingActiveDOMObjects; }
bool hasPendingActivity() const;
+ void removeFromContextsMap();
private:
// The following addMessage function is deprecated.
@@ -328,6 +336,7 @@
#endif
String m_domainForCachePartition;
+ mutable ScriptExecutionContextIdentifier m_contextIdentifier;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.cpp (229615 => 229616)
--- trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2018-03-14 22:32:28 UTC (rev 229615)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2018-03-14 22:52:09 UTC (rev 229616)
@@ -89,6 +89,8 @@
WorkerGlobalScope::~WorkerGlobalScope()
{
ASSERT(thread().thread() == &Thread::current());
+ // We need to remove from the contexts map very early in the destructor so that calling postTask() on this WorkerGlobalScope from another thread is safe.
+ removeFromContextsMap();
m_performance = nullptr;
m_crypto = nullptr;
