Changeset 122198 in webkit
- Timestamp:
- Jul 10, 2012 12:08:53 AM (12 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 2 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r122197 r122198 1 2012-07-09 Gavin Barraclough <barraclough@apple.com> 2 3 Threadsafety issues in WebScriptObject 4 https://bugs.webkit.org/show_bug.cgi?id=90849 5 6 Reviewed by Filip Pizlo. 7 8 WebScriptObject maintains a NSMap of wrapper objects. A race condition exists 9 between a wrapper being retrieved from the map, and being released - if the 10 final release on an object is called between a call to getJSWrapper and the 11 subsequent retain, we may end up with a stale object reference. 12 13 We can make this safe by hoisting the removal from the map from delloc up into 14 release (if the retainCount is 1), and locking release against retrieval from 15 the map. Since release may be called from another thread, and NSMap is not 16 threadsafe, we'd better lock around all access to the map (this fix already 17 necessitates get & remove to be locked, so this just adds 'add', too). 18 19 * bindings/objc/WebScriptObject.mm: 20 (WebCore::createJSWrapper): 21 - lock around getJSWrapper, retain. 22 (-[WebScriptObject _setImp:originRootObject:rootObject:]): 23 - lock around addJSWrapper. 24 (-[WebScriptObject release]): 25 - Added; removeJSWrapper for last release, lock & synchronized vs. getJSWrapper. 26 (-[WebScriptObject dealloc]): 27 - removeJSWrapper call hoisted into release. 28 1 29 2012-07-09 Christophe Dumez <christophe.dumez@intel.com> 2 30 -
trunk/Source/WebCore/bindings/objc/WebScriptObject.mm
r121381 r122198 85 85 id createJSWrapper(JSC::JSObject* object, PassRefPtr<JSC::Bindings::RootObject> origin, PassRefPtr<JSC::Bindings::RootObject> root) 86 86 { 87 // NSMap is not thread safe, hold the JSC API lock; also synchronize this vs. release. 88 JSC::JSLockHolder holder(JSDOMWindowBase::commonJSGlobalData()); 89 87 90 if (id wrapper = getJSWrapper(object)) 88 91 return [[wrapper retain] autorelease]; … … 146 149 _private->originRootObject = originRootObject.leakRef(); 147 150 151 // NSMap is not thread safe, hold the JSC API lock. 152 JSC::JSLockHolder holder(JSDOMWindowBase::commonJSGlobalData()); 153 148 154 WebCore::addJSWrapper(self, imp); 149 155 … … 224 230 } 225 231 232 - (oneway void)release 233 { 234 { 235 // NSMap is not thread safe, hold the JSC API lock; also synchronize this vs. getJSWrapper. 236 JSC::JSLockHolder holder(JSDOMWindowBase::commonJSGlobalData()); 237 238 // If we're releasing the last reference to this object, remove if from the map, 239 // this will prevent this object from being returned by getJSWrapper. 240 if (_private->imp && [self retainCount] == 1) 241 WebCore::removeJSWrapper(_private->imp); 242 } 243 244 [super release]; 245 } 246 226 247 - (void)dealloc 227 248 { 228 249 if (WebCoreObjCScheduleDeallocateOnMainThread([WebScriptObject class], self)) 229 250 return; 230 231 if (_private->imp)232 WebCore::removeJSWrapper(_private->imp);233 251 234 252 if (_private->rootObject && _private->rootObject->isValid())
Note: See TracChangeset
for help on using the changeset viewer.