Changeset 244323 in webkit


Ignore:
Timestamp:
Apr 15, 2019 7:12:26 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

Unreviewed, rolling out r243672.
https://bugs.webkit.org/show_bug.cgi?id=196952

[JSValue release] should be thread-safe (Requested by
yusukesuzuki on #webkit).

Reverted changeset:

"[JSC] JSWrapperMap should not use Objective-C Weak map
(NSMapTable with NSPointerFunctionsWeakMemory) for
m_cachedObjCWrappers"
https://bugs.webkit.org/show_bug.cgi?id=196392
https://trac.webkit.org/changeset/243672

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSContext.mm

    r243672 r244323  
    370370}
    371371
    372 - (void)removeWrapper:(JSValue *)value
    373 {
    374     return [[self wrapperMap] removeWrapper:value];
    375 }
    376 
    377372+ (JSContext *)contextWithJSGlobalContextRef:(JSGlobalContextRef)globalContext
    378373{
  • trunk/Source/JavaScriptCore/API/JSContextInternal.h

    r243672 r244323  
    5555- (JSValue *)wrapperForObjCObject:(id)object;
    5656- (JSValue *)wrapperForJSObject:(JSValueRef)value;
    57 - (void)removeWrapper:(JSValue *)value;
    5857
    5958@end
  • trunk/Source/JavaScriptCore/API/JSValue.mm

    r243672 r244323  
    7272- (void)dealloc
    7373{
    74     [_context removeWrapper:self];
    7574    JSValueUnprotect([_context JSGlobalContextRef], m_value);
    7675    [_context release];
     
    10771076        return nil;
    10781077
    1079     ASSERT(context);
    10801078    _context = [context retain];
    10811079    m_value = value;
  • trunk/Source/JavaScriptCore/API/JSWrapperMap.h

    r243672 r244323  
    3838- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:(JSContext *)context;
    3939
    40 - (void)removeWrapper:(JSValue *)wrapper;
    41 
    4240@end
    4341
  • trunk/Source/JavaScriptCore/API/JSWrapperMap.mm

    r243672 r244323  
    582582@end
    583583
    584 struct WrapperKey {
    585     static constexpr uintptr_t hashTableDeletedValue() { return 1; }
    586 
    587     WrapperKey() = default;
    588 
    589     explicit WrapperKey(WTF::HashTableDeletedValueType)
    590         : m_wrapper(reinterpret_cast<JSValue *>(hashTableDeletedValue()))
    591     {
    592     }
    593 
    594     explicit WrapperKey(JSValue *wrapper)
    595         : m_wrapper(wrapper)
    596     {
    597     }
    598 
    599     bool isHashTableDeletedValue() const
    600     {
    601         return reinterpret_cast<uintptr_t>(m_wrapper) == hashTableDeletedValue();
    602     }
    603 
    604     __unsafe_unretained JSValue *m_wrapper { nil };
    605 
    606     struct Hash {
    607         static unsigned hash(const WrapperKey& key)
    608         {
    609             return DefaultHash<JSValueRef>::Hash::hash([key.m_wrapper JSValueRef]);
    610         }
    611 
    612         static bool equal(const WrapperKey& lhs, const WrapperKey& rhs)
    613         {
    614             return lhs.m_wrapper == rhs.m_wrapper;
    615         }
    616 
    617         static const bool safeToCompareToEmptyOrDeleted = false;
    618     };
    619 
    620     struct Traits : public SimpleClassHashTraits<WrapperKey> {
    621         static const bool hasIsEmptyValueFunction = true;
    622         static bool isEmptyValue(const WrapperKey& key)
    623         {
    624             return key.m_wrapper == nullptr;
    625         }
    626     };
    627 
    628     struct Translator {
    629         struct ValueAndContext {
    630             __unsafe_unretained JSContext *m_context;
    631             JSValueRef m_value;
    632         };
    633 
    634         static unsigned hash(const ValueAndContext& value)
    635         {
    636             return DefaultHash<JSValueRef>::Hash::hash(value.m_value);
    637         }
    638 
    639         static bool equal(const WrapperKey& lhs, const ValueAndContext& value)
    640         {
    641             return [lhs.m_wrapper JSValueRef] == value.m_value;
    642         }
    643 
    644         static void translate(WrapperKey& result, const ValueAndContext& value, unsigned)
    645         {
    646             result = WrapperKey([[[JSValue alloc] initWithValue:value.m_value inContext:value.m_context] autorelease]);
    647         }
    648     };
    649 };
    650 
    651584@implementation JSWrapperMap {
    652585    NSMutableDictionary *m_classMap;
    653586    std::unique_ptr<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>> m_cachedJSWrappers;
    654     HashSet<WrapperKey, WrapperKey::Hash, WrapperKey::Traits> m_cachedObjCWrappers;
     587    NSMapTable *m_cachedObjCWrappers;
    655588}
    656589
     
    660593    if (!self)
    661594        return nil;
     595
     596    NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
     597    NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
     598    m_cachedObjCWrappers = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
    662599
    663600    m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>>(toJS(context)->vm());
     
    671608- (void)dealloc
    672609{
     610    [m_cachedObjCWrappers release];
    673611    [m_classMap release];
    674612    [super dealloc];
     
    725663{
    726664    ASSERT(toJSGlobalObject([context JSGlobalContextRef])->wrapperMap() == self);
    727     WrapperKey::Translator::ValueAndContext valueAndContext { context, value };
    728     auto addResult = m_cachedObjCWrappers.add<WrapperKey::Translator>(valueAndContext);
    729     return addResult.iterator->m_wrapper;
    730 }
    731 
    732 - (void)removeWrapper:(JSValue *)wrapper
    733 {
    734     m_cachedObjCWrappers.remove(WrapperKey(wrapper));
     665    JSValue *wrapper = (__bridge JSValue *)NSMapGet(m_cachedObjCWrappers, value);
     666    if (!wrapper) {
     667        wrapper = [[[JSValue alloc] initWithValue:value inContext:context] autorelease];
     668        NSMapInsert(m_cachedObjCWrappers, value, (__bridge void*)wrapper);
     669    }
     670    return wrapper;
    735671}
    736672
  • trunk/Source/JavaScriptCore/API/tests/testapi.mm

    r243672 r244323  
    566566{
    567567    @autoreleasepool {
    568         JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
    569         JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
     568        JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
     569        JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
    570570        [context evaluateScript:@"bad"];
    571     }
    572 
    573     @autoreleasepool {
    574         JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
    575         JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
    576         JSValue *number1 = [context evaluateScript:@"42092389"];
    577         JSValue *number2 = [context evaluateScript:@"42092389"];
    578         checkResult(@"wrapper cache for numbers", number1 == number2 && number1.isNumber && [number1 toInt32] == 42092389);
    579     }
    580 
    581     @autoreleasepool {
    582         JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
    583         JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
    584         JSValue *object1 = [context evaluateScript:@"({})"];
    585         JSValue *object2 = [context evaluateScript:@"({})"];
    586         checkResult(@"wrapper cache for objects", object1 != object2);
    587571    }
    588572
  • trunk/Source/JavaScriptCore/ChangeLog

    r244314 r244323  
     12019-04-15  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r243672.
     4        https://bugs.webkit.org/show_bug.cgi?id=196952
     5
     6        [JSValue release] should be thread-safe (Requested by
     7        yusukesuzuki on #webkit).
     8
     9        Reverted changeset:
     10
     11        "[JSC] JSWrapperMap should not use Objective-C Weak map
     12        (NSMapTable with NSPointerFunctionsWeakMemory) for
     13        m_cachedObjCWrappers"
     14        https://bugs.webkit.org/show_bug.cgi?id=196392
     15        https://trac.webkit.org/changeset/243672
     16
    1172019-04-15  Saam barati  <sbarati@apple.com>
    218
Note: See TracChangeset for help on using the changeset viewer.