⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 281160 in webkit


Ignore:
Timestamp:
Aug 17, 2021, 1:32:30 PM (5 years ago)
Author:
ddkilzer@apple.com
Message:

[Metal ANGLE] Fix over-autorelease of rx::DisplayMtl::getMetalDeviceMatchingAttribute() and various Objective-C leaks
<​https://webkit.org/b/229128>
<rdar://problem/81964007>

Reviewed by Alex Christensen.

  • src/libANGLE/renderer/metal/DisplayMtl.h:

(rx::DisplayMtl::getMetalDeviceMatchingAttribute):

  • Change to return mtl::AutoObjCPtr<> to make ownership clear.

(rx::DisplayMtl::mMetalDevice):

  • No need to initialize to nil.
  • src/libANGLE/renderer/metal/DisplayMtl.mm:

(rx::DisplayMtl::initializeImpl):

  • Update for changes to getMetalDeviceMatchingAttribute().

(rx::DisplayMtl::getMetalDeviceMatchingAttribute):

  • Change to return mtl::AutoObjCPtr<> to make ownership clear.
  • Fix leak of deviceList, externalGPUs, integratedGPUs, and discreteGPUs.
  • Use mtl::adoptObjCObj<>() to prevent leak of MTLCreateSystemDefaultDevice().
  • src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm:
  • Fix leak of captureDescriptor in two different if blocks.
  • src/libANGLE/renderer/metal/ProgramMtl.mm:
  • Fix leak of funcConstants in early return on error path.
  • src/libANGLE/renderer/metal/ProvokingVertexHelper.mm:

(rx::ProvokingVertexHelper::getSpecializedShader):

  • Fix leak of fcValues.
  • src/libANGLE/renderer/metal/SurfaceMtl.mm:
  • Fix leak of captureDescriptor in two different if blocks.
  • src/libANGLE/renderer/metal/mtl_common.h:

(rx::mtl::WrappedObject::retainAssign):

  • Move statement inside #if/#endif that isn't needed for ARC.

(rx::mtl::WrappedObject::unretainAssign): Add.
(rx::mtl::AutoObjCPtr::AutoObjCPtr): Add.
(rx::mtl::adoptObjCObj): Add.

  • Add a helper method to adopt an Objective-C object to eliminate the need to autorelease a +1 retained object before an mtl::AutoObjCPtr<> object wraps it. Modeled after WTF::RetainPtr<> in WebKit.
  • src/libANGLE/renderer/metal/mtl_state_cache.mm:

(rx::mtl::RenderPipelineCache::createRenderPipelineState):
(rx::mtl::ProvokingVertexComputePipelineCache::createComputePipelineState):

  • Use adoptObjCObj<>() to fix potential leak on the early return path since these methods return an mtl::AutoObjCPtr<>.
Location:
trunk/Source/ThirdParty/ANGLE
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/ThirdParty/ANGLE/ChangeLog

    r281088 r281160  
     12021-08-17  David Kilzer  <ddkilzer@apple.com>
     2
     3        [Metal ANGLE] Fix over-autorelease of rx::DisplayMtl::getMetalDeviceMatchingAttribute() and various Objective-C leaks
     4        <https://webkit.org/b/229128>
     5        <rdar://problem/81964007>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * src/libANGLE/renderer/metal/DisplayMtl.h:
     10        (rx::DisplayMtl::getMetalDeviceMatchingAttribute):
     11        - Change to return mtl::AutoObjCPtr<> to make ownership clear.
     12        (rx::DisplayMtl::mMetalDevice):
     13        - No need to initialize to nil.
     14
     15        * src/libANGLE/renderer/metal/DisplayMtl.mm:
     16        (rx::DisplayMtl::initializeImpl):
     17        - Update for changes to getMetalDeviceMatchingAttribute().
     18        (rx::DisplayMtl::getMetalDeviceMatchingAttribute):
     19        - Change to return mtl::AutoObjCPtr<> to make ownership clear.
     20        - Fix leak of `deviceList`, `externalGPUs`, `integratedGPUs`, and
     21          `discreteGPUs`.
     22        - Use mtl::adoptObjCObj<>() to prevent leak of
     23          MTLCreateSystemDefaultDevice().
     24
     25        * src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm:
     26        - Fix leak of `captureDescriptor` in two different if blocks.
     27
     28        * src/libANGLE/renderer/metal/ProgramMtl.mm:
     29        - Fix leak of `funcConstants` in early return on error path.
     30
     31        * src/libANGLE/renderer/metal/ProvokingVertexHelper.mm:
     32        (rx::ProvokingVertexHelper::getSpecializedShader):
     33        - Fix leak of `fcValues`.
     34
     35        * src/libANGLE/renderer/metal/SurfaceMtl.mm:
     36        - Fix leak of `captureDescriptor` in two different if blocks.
     37
     38        * src/libANGLE/renderer/metal/mtl_common.h:
     39        (rx::mtl::WrappedObject::retainAssign):
     40        - Move statement inside #if/#endif that isn't needed for ARC.
     41        (rx::mtl::WrappedObject::unretainAssign): Add.
     42        (rx::mtl::AutoObjCPtr::AutoObjCPtr): Add.
     43        (rx::mtl::adoptObjCObj): Add.
     44        - Add a helper method to adopt an Objective-C object to
     45          eliminate the need to autorelease a +1 retained object before
     46          an mtl::AutoObjCPtr<> object wraps it.  Modeled after
     47          WTF::RetainPtr<> in WebKit.
     48
     49        * src/libANGLE/renderer/metal/mtl_state_cache.mm:
     50        (rx::mtl::RenderPipelineCache::createRenderPipelineState):
     51        (rx::mtl::ProvokingVertexComputePipelineCache::createComputePipelineState):
     52        - Use adoptObjCObj<>() to fix potential leak on the early return
     53          path since these methods return an mtl::AutoObjCPtr<>.
     54
    1552021-08-16  David Kilzer  <ddkilzer@apple.com>
    256

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.h

    r279143 r281160  
    176176    void initializeLimitations();
    177177    EGLenum EGLDrawingBufferTextureTarget();
    178     id<MTLDevice> getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs);
     178    mtl::AutoObjCPtr<id<MTLDevice>> getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs);
    179179    angle::Result initializeShaderLibrary();
    180180
    181     mtl::AutoObjCPtr<id<MTLDevice>> mMetalDevice = nil;
     181    mtl::AutoObjCPtr<id<MTLDevice>> mMetalDevice;
    182182    uint32_t mMetalDeviceVendorId                = 0;
    183183

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm

    r280010 r281160  
    140140    ANGLE_MTL_OBJC_SCOPE
    141141    {
    142         mMetalDevice = [getMetalDeviceMatchingAttribute(display->getAttributeMap()) ANGLE_MTL_AUTORELEASE];
     142        mMetalDevice = getMetalDeviceMatchingAttribute(display->getAttributeMap());
    143143        //If we can't create a device, fail initialization.
    144144        if(!mMetalDevice.get())
    … …  
    245245
    246246
    247 id<MTLDevice> DisplayMtl::getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs)
     247mtl::AutoObjCPtr<id<MTLDevice>> DisplayMtl::getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs)
    248248{
    249249#if defined(ANGLE_PLATFORM_MACOS) || defined(ANGLE_PLATFORM_MACCATALYST)
    250250    const std::string anglePreferredDevice = angle::GetEnvironmentVar(kANGLEPreferredDeviceEnv);
    251     NSArray<id<MTLDevice>> *deviceList = MTLCopyAllDevices();
     251    auto deviceList = mtl::adoptObjCObj(MTLCopyAllDevices());
    252252    if (anglePreferredDevice != "")
    253253    {
    254         for (id<MTLDevice> device in deviceList)
     254        for (id<MTLDevice> device in deviceList.get())
    255255        {
    256256            if ([device.name.lowercaseString
    … …  
    259259            {
    260260                NSLog(@"Using Metal Device: %@", [device name]);
    261                 return  device;
     261                return device;
    262262                break;
    263263            }
    … …  
    265265    }
    266266
    267     NSMutableArray<id<MTLDevice>> *externalGPUs = [[NSMutableArray alloc] init];
    268     NSMutableArray<id<MTLDevice>> *integratedGPUs = [[NSMutableArray alloc] init];
    269     NSMutableArray<id<MTLDevice>> *discreteGPUs = [[NSMutableArray alloc] init];
    270     for (id <MTLDevice> device in deviceList) {
     267    auto externalGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
     268    auto integratedGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
     269    auto discreteGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
     270    for (id <MTLDevice> device in deviceList.get()) {
    271271        if (device.removable)
    272272        {
    … …  
    287287    {
    288288        //Search for a discrete GPU first.
    289         for (id<MTLDevice> device in discreteGPUs) {
     289        for (id<MTLDevice> device in discreteGPUs.get()) {
    290290            if(![device isHeadless])
    291291                return device;
    … …  
    293293    }
    294294    //If we've selected a low power device, look through integrated devices.
    295     for (id<MTLDevice> device in integratedGPUs) {
     295    for (id<MTLDevice> device in integratedGPUs.get()) {
    296296        if(![device isHeadless])
    297297            return device;
    298298    }
    299299    //If we selected a low power device and there's no low-power devices avaialble, return the first (default) device.
    300     if(deviceList.count > 0)
     300    if (deviceList.get().count > 0)
    301301        return deviceList[0];
    302302#endif
    303303    //If we can't find anything, or are on a platform that doesn't support power options, create a default device.
    304     return MTLCreateSystemDefaultDevice();
     304    return mtl::adoptObjCObj(MTLCreateSystemDefaultDevice());
    305305}
    306306

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm

    r273516 r281160  
    169169    if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
    170170    {
    171         MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
    172         captureDescriptor.captureObject        = metalDevice;
    173         const std::string filePath              = GetMetalCaptureFile();
     171        auto captureDescriptor                = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
     172        captureDescriptor.get().captureObject = metalDevice;
     173        const std::string filePath            = GetMetalCaptureFile();
    174174        if (filePath != "")
    175175        {
    176176            const std::string numberedPath =
    177177                filePath + std::to_string(gFrameCaptured - 1) + ".gputrace";
    178             captureDescriptor.destination = MTLCaptureDestinationGPUTraceDocument;
    179             captureDescriptor.outputURL =
     178            captureDescriptor.get().destination = MTLCaptureDestinationGPUTraceDocument;
     179            captureDescriptor.get().outputURL =
    180180                [NSURL fileURLWithPath:[NSString stringWithUTF8String:numberedPath.c_str()]
    181181                           isDirectory:false];
    … …  
    184184        {
    185185            // This will pause execution only if application is being debugged inside Xcode
    186             captureDescriptor.destination = MTLCaptureDestinationDeveloperTools;
     186            captureDescriptor.get().destination = MTLCaptureDestinationDeveloperTools;
    187187        }
    188188
    189189        NSError *error;
    190         if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
     190        if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
    191191        {
    192192            NSLog(@"Failed to start capture, error %@", error);
    … …  
    197197        if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
    198198    {
    199         MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
    200         captureDescriptor.captureObject        = metalDevice;
     199        auto captureDescriptor                = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
     200        captureDescriptor.get().captureObject = metalDevice;
    201201
    202202        NSError *error;
    203         if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
     203        if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
    204204        {
    205205            NSLog(@"Failed to start capture, error %@", error);

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm

    r279606 r281160  
    599599    mtl::TranslatedShaderInfo *translatedMslInfo = &mMslShaderTranslateInfo[shaderType];
    600600    ProgramShaderObjVariantMtl *shaderVariant;
    601     MTLFunctionConstantValues *funcConstants = nil;
     601    mtl::AutoObjCObj<MTLFunctionConstantValues> funcConstants;
    602602
    603603    if (shaderType == gl::ShaderType::Vertex)
    … …  
    635635            }
    636636
    637             funcConstants = [[MTLFunctionConstantValues alloc] init];
     637            funcConstants = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
    638638            [funcConstants setConstantValue:&emulateDiscard
    639639                                       type:MTLDataTypeBool
    … …  
    673673            }
    674674
    675             funcConstants = [[MTLFunctionConstantValues alloc] init];
     675            funcConstants = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
    676676            [funcConstants setConstantValue:&emulateCoverageMask
    677677                                       type:MTLDataTypeBool
    … …  
    695695    ANGLE_MTL_OBJC_SCOPE
    696696    {
    697         ANGLE_TRY(CreateMslShader(context, translatedMslInfo->metalLibrary, SHADER_ENTRY_NAME, funcConstants,
     697        ANGLE_TRY(CreateMslShader(context, translatedMslInfo->metalLibrary, SHADER_ENTRY_NAME, funcConstants.get(),
    698698                                  &shaderVariant->metalShader));
    699         [funcConstants ANGLE_MTL_AUTORELEASE];
    700699    }
    701700

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.mm

    r277741 r281160  
    175175{
    176176    uint indexBufferKey = buildIndexBufferKey(pipelineDesc);
    177     MTLFunctionConstantValues  * fcValues = [[MTLFunctionConstantValues alloc]init];
     177    auto fcValues = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
    178178    [fcValues setConstantValue:&indexBufferKey type:MTLDataTypeUInt withName:@"fixIndexBufferKey"];
    179179   
    180     return CreateMslShader(context, mProvokingVertexLibrary, @"fixIndexBuffer", fcValues, shaderOut);
    181 }
     180    return CreateMslShader(context, mProvokingVertexLibrary, @"fixIndexBuffer", fcValues.get(), shaderOut);
     181}
     182
    182183//Private command buffer
    183184bool ProvokingVertexHelper::hasSpecializedShader(gl::ShaderType shaderType,

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/SurfaceMtl.mm

    r277824 r281160  
    178178    if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
    179179    {
    180         MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
    181         captureDescriptor.captureObject        = metalDevice;
    182         const std::string filePath              = GetMetalCaptureFile();
     180        auto captureDescriptor                = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
     181        captureDescriptor.get().captureObject = metalDevice;
     182        const std::string filePath            = GetMetalCaptureFile();
    183183        if (filePath != "")
    184184        {
    185185            const std::string numberedPath =
    186186                filePath + std::to_string(gFrameCaptured - 1) + ".gputrace";
    187             captureDescriptor.destination = MTLCaptureDestinationGPUTraceDocument;
    188             captureDescriptor.outputURL =
     187            captureDescriptor.get().destination = MTLCaptureDestinationGPUTraceDocument;
     188            captureDescriptor.get().outputURL =
    189189                [NSURL fileURLWithPath:[NSString stringWithUTF8String:numberedPath.c_str()]
    190190                           isDirectory:false];
    … …  
    193193        {
    194194            // This will pause execution only if application is being debugged inside Xcode
    195             captureDescriptor.destination = MTLCaptureDestinationDeveloperTools;
     195            captureDescriptor.get().destination = MTLCaptureDestinationDeveloperTools;
    196196        }
    197197
    198198        NSError *error;
    199         if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
     199        if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
    200200        {
    201201            NSLog(@"Failed to start capture, error %@", error);
    … …  
    206206        if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
    207207    {
    208         MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
    209         captureDescriptor.captureObject        = metalDevice;
     208        auto captureDescriptor                = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
     209        captureDescriptor.get().captureObject = metalDevice;
    210210
    211211        NSError *error;
    212         if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
     212        if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
    213213        {
    214214            NSLog(@"Failed to start capture, error %@", error);

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_common.h

    r279143 r281160  
    209209    void retainAssign(T obj)
    210210    {
     211#if !__has_feature(objc_arc)
    211212        T retained = obj;
    212 #if !__has_feature(objc_arc)
    213213        [retained retain];
    214214#endif
    … …  
    217217    }
    218218
     219    void unretainAssign(T obj)
     220    {
     221        release();
     222        mMetalObject = obj;
     223    }
     224
    219225  private:
    220226    void release()
    … …  
    228234    T mMetalObject = nil;
    229235};
     236
     237// Because ARC enablement is a compile-time choice, and we compile this header
     238// both ways, we need a separate copy of our code when ARC is enabled.
     239#if __has_feature(objc_arc)
     240#define adoptObjCObj adoptObjCObjArc
     241#endif
     242
     243template <typename T>
     244class AutoObjCPtr;
     245
     246template <typename T>
     247using AutoObjCObj = AutoObjCPtr<T *>;
     248
     249template <typename U>
     250AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT) __attribute__((__warn_unused_result__));
    230251
    231252// This class is similar to WrappedObject, however, it allows changing the
    … …  
    294315    using ParentType::retainAssign;
    295316
     317    template <typename U>
     318    friend AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT) __attribute__((__warn_unused_result__));
     319
    296320  private:
     321    enum AdoptTag { Adopt };
     322    AutoObjCPtr(T src, AdoptTag) { this->unretainAssign(src); }
     323
    297324    void transfer(AutoObjCPtr &&src)
    298325    {
    … …  
    302329};
    303330
    304 template <typename T>
    305 using AutoObjCObj = AutoObjCPtr<T *>;
     331template <typename U>
     332inline AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT src)
     333{
     334#if __has_feature(objc_arc)
     335    return src;
     336#elif defined(OBJC_NO_GC)
     337    return AutoObjCPtr<U*>(src, AutoObjCPtr<U*>::Adopt);
     338#else
     339#error "ObjC GC not supported."
     340#endif
     341}
     342
    306343
    307344// NOTE: SharedEvent is only declared on iOS 12.0+ or mac 10.14+

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_state_cache.mm

    r277741 r281160  
    10421042        // Create pipeline state
    10431043        NSError *err = nil;
    1044         id<MTLRenderPipelineState> newState =
    1045             [metalDevice newRenderPipelineStateWithDescriptor:objCDesc error:&err];
     1044        auto newState =
     1045            adoptObjCObj([metalDevice newRenderPipelineStateWithDescriptor:objCDesc error:&err]);
    10461046        if (err)
    10471047        {
    … …  
    10501050        }
    10511051
    1052         return [newState ANGLE_MTL_AUTORELEASE];
     1052        return newState;
    10531053    }
    10541054}
    … …  
    12341234        // Convert to Objective-C desc:
    12351235        NSError *err = nil;
    1236         id<MTLComputePipelineState> newState =
    1237             [metalDevice newComputePipelineStateWithFunction:computeFunction error:&err];
     1236        auto newState =
     1237            adoptObjCObj([metalDevice newComputePipelineStateWithFunction:computeFunction error:&err]);
    12381238        if (err)
    12391239        {
    … …  
    12421242        }
    12431243
    1244         return [newState ANGLE_MTL_AUTORELEASE];
     1244        return newState;
    12451245    }
    12461246}
Note: See TracChangeset for help on using the changeset viewer.