Bug 86648

Summary: WebKit2: Make PluginInfoStore properly thread-safe.
Product: WebKit Reporter: Andreas Kling <kling>
Component: Plug-insAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Andreas Kling 2012-05-16 10:09:09 PDT
To stay out of trouble, we should make sure that the internal storage of PluginInfoStore is thread-isolated.
Comment 1 Andreas Kling 2012-05-16 10:33:53 PDT
Created attachment 142293 [details]
Patch
Comment 2 Andreas Kling 2012-05-16 13:16:36 PDT
Running layout tests just caught a crash that's fixed by this:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff9583dce2 __pthread_kill + 10
1   libsystem_c.dylib             	0x00007fff8f2c07d2 pthread_kill + 95
2   libsystem_c.dylib             	0x00007fff8f2b1a7a abort + 143
3   libsystem_c.dylib             	0x00007fff8f31084c free + 389
4   com.apple.JavaScriptCore      	0x0000000102d3b6a2 WTF::fastFree(void*) + 98
5   com.apple.WebKit2             	0x0000000101a9c963 WTF::StringImpl::operator delete(void*) + 35 (StringImpl.h:66)
6   com.apple.WebKit2             	0x0000000101a9c922 WTF::StringImpl::deref() + 66 (StringImpl.h:396)
7   com.apple.WebKit2             	0x0000000101a9c8d2 void WTF::derefIfNotNull<WTF::StringImpl>(WTF::StringImpl*) + 50 (PassRefPtr.h:53)
8   com.apple.WebKit2             	0x0000000101a9d948 WTF::RefPtr<WTF::StringImpl>::~RefPtr() + 24 (RefPtr.h:56)
9   com.apple.WebKit2             	0x0000000101a9d925 WTF::RefPtr<WTF::StringImpl>::~RefPtr() + 21 (RefPtr.h:56)
10  com.apple.WebKit2             	0x0000000101a9d905 WTF::String::~String() + 21 (WTFString.h:135)
11  com.apple.WebKit2             	0x0000000101a9c415 WTF::String::~String() + 21 (WTFString.h:135)
12  com.apple.WebKit2             	0x0000000101ac394c WebCore::MimeClassInfo::~MimeClassInfo() + 60 (PluginData.h:32)
13  com.apple.WebKit2             	0x0000000101abe305 WebCore::MimeClassInfo::~MimeClassInfo() + 21 (PluginData.h:32)
14  com.apple.WebKit2             	0x0000000101ac38ef WTF::VectorDestructor<true, WebCore::MimeClassInfo>::destruct(WebCore::MimeClassInfo*, WebCore::MimeClassInfo*) + 47 (Vector.h:57)
15  com.apple.WebKit2             	0x0000000101ac38ad WTF::VectorTypeOperations<WebCore::MimeClassInfo>::destruct(WebCore::MimeClassInfo*, WebCore::MimeClassInfo*) + 29 (Vector.h:221)
16  com.apple.WebKit2             	0x0000000101ac37b4 WTF::Vector<WebCore::MimeClassInfo, 0ul>::shrink(unsigned long) + 148 (Vector.h:882)
17  com.apple.WebKit2             	0x0000000101ac36ac WTF::Vector<WebCore::MimeClassInfo, 0ul>::~Vector() + 44 (Vector.h:510)
18  com.apple.WebKit2             	0x0000000101ac3675 WTF::Vector<WebCore::MimeClassInfo, 0ul>::~Vector() + 21 (Vector.h:511)
19  com.apple.WebKit2             	0x0000000101ac359c WebCore::PluginInfo::~PluginInfo() + 28 (PluginData.h:53)
20  com.apple.WebKit2             	0x0000000101abe325 WebCore::PluginInfo::~PluginInfo() + 21 (PluginData.h:53)
21  com.apple.WebKit2             	0x0000000101c24f3f WTF::VectorDestructor<true, WebCore::PluginInfo>::destruct(WebCore::PluginInfo*, WebCore::PluginInfo*) + 47 (Vector.h:57)
22  com.apple.WebKit2             	0x0000000101c24efd WTF::VectorTypeOperations<WebCore::PluginInfo>::destruct(WebCore::PluginInfo*, WebCore::PluginInfo*) + 29 (Vector.h:221)
23  com.apple.WebKit2             	0x0000000101c24e44 WTF::Vector<WebCore::PluginInfo, 0ul>::shrink(unsigned long) + 148 (Vector.h:882)
24  com.apple.WebKit2             	0x0000000101c24d94 WTF::Vector<WebCore::PluginInfo, 0ul>::~Vector() + 52 (Vector.h:511)
25  com.apple.WebKit2             	0x0000000101c1bdf5 WTF::Vector<WebCore::PluginInfo, 0ul>::~Vector() + 21 (Vector.h:511)
26  com.apple.WebKit2             	0x0000000101c241f2 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebContext::*)(unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul> const&)>, void ()(WebKit::WebContext*, unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul>)>::~BoundFunctionImpl() + 66 (Functional.h:455)
27  com.apple.WebKit2             	0x0000000101c24055 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebContext::*)(unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul> const&)>, void ()(WebKit::WebContext*, unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul>)>::~BoundFunctionImpl() + 21 (Functional.h:455)
28  com.apple.WebKit2             	0x0000000101c24079 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebContext::*)(unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul> const&)>, void ()(WebKit::WebContext*, unsigned long long, WTF::Vector<WebCore::PluginInfo, 0ul>)>::~BoundFunctionImpl() + 25 (Functional.h:453)
29  com.apple.WebCore             	0x0000000104f62be3 WTF::ThreadSafeRefCounted<WTF::FunctionImplBase>::deref() + 83 (ThreadSafeRefCounted.h:138)
30  com.apple.WebCore             	0x0000000104f62b7b void WTF::derefIfNotNull<WTF::FunctionImplBase>(WTF::FunctionImplBase*) + 59 (PassRefPtr.h:53)
31  com.apple.WebCore             	0x0000000104f62b38 WTF::RefPtr<WTF::FunctionImplBase>::~RefPtr() + 24 (RefPtr.h:56)
32  com.apple.WebCore             	0x0000000104f62b15 WTF::RefPtr<WTF::FunctionImplBase>::~RefPtr() + 21 (RefPtr.h:56)
33  com.apple.WebCore             	0x0000000104f62af5 WTF::FunctionBase::~FunctionBase() + 21 (Functional.h:568)
34  com.apple.WebCore             	0x0000000104f62ad5 WTF::Function<void ()()>::~Function() + 21 (Functional.h:595)
35  com.apple.WebCore             	0x0000000104f62ab5 WTF::Function<void ()()>::~Function() + 21 (Functional.h:595)
36  com.apple.WebCore             	0x0000000104f630df WTF::VectorDestructor<true, WTF::Function<void ()()> >::destruct(WTF::Function<void ()()>*, WTF::Function<void ()()>*) + 47 (Vector.h:57)
37  com.apple.WebCore             	0x0000000104f6309d WTF::VectorTypeOperations<WTF::Function<void ()()> >::destruct(WTF::Function<void ()()>*, WTF::Function<void ()()>*) + 29 (Vector.h:221)
38  com.apple.WebCore             	0x0000000104f62fe4 WTF::Vector<WTF::Function<void ()()>, 0ul>::shrink(unsigned long) + 148 (Vector.h:882)
39  com.apple.WebCore             	0x0000000104f62f34 WTF::Vector<WTF::Function<void ()()>, 0ul>::~Vector() + 52 (Vector.h:511)
40  com.apple.WebCore             	0x0000000104f62315 WTF::Vector<WTF::Function<void ()()>, 0ul>::~Vector() + 21 (Vector.h:511)
41  com.apple.WebCore             	0x0000000104f620a3 WebCore::RunLoop::performWork() + 163 (RunLoop.cpp:68)
42  com.apple.WebCore             	0x000000010540ec4e WebCore::RunLoop::performWork(void*) + 62 (RunLoopCF.cpp:66)
43  com.apple.CoreFoundation      	0x00007fff939034f1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
Comment 3 Darin Adler 2012-05-17 09:26:49 PDT
Comment on attachment 142293 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142293&action=review

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.cpp:79
> +static inline Vector<PluginModuleInfo> deepCopyPluginInfoVector(const Vector<PluginModuleInfo>& vector)
> +{
> +    // Let the copy begin!
> +    Vector<PluginModuleInfo> copy;
> +    for (unsigned i = 0; i < vector.size(); ++i)
> +        copy.append(vector[i].isolatedCopy());
> +    return copy;
> +}

I think this function needs the word isolated in its title.

Would be nice to do a reserveCapacity here so we get a vector of the right size instead of a larger one.
Comment 4 Andreas Kling 2012-05-17 11:54:11 PDT
Committed r117471: <http://trac.webkit.org/changeset/117471>