Skip to content

Commit b327848

Browse files
committed
record/avfoundation: plug MRC leaks of AVAssetWriter objects and GCD queue
record_avfoundation_t holds five Objective-C object pointers as plain C struct members: AVAssetWriter *assetWriter; AVAssetWriterInput *videoInput; AVAssetWriterInput *audioInput; AVAssetWriterInputPixelBufferAdaptor *pixelBufferAdaptor; dispatch_queue_t encodingQueue; All five are populated via alloc+init (or dispatch_queue_create), each of which hands back a +1-retained object. avfoundation_release_handle() cleared these fields with plain handle->assetWriter = nil; Under ARC that is fine because modern clang auto-qualifies ObjC struct members as __strong and the assignment releases the prior value. Under MRC the assignment is a bare pointer overwrite - the previously- retained object is never released. RetroArch builds .m files without -fobjc-arc in the top-level qb Makefile and in the RetroArch/_Metal/_OSX107/_iOS13 Xcode projects (only _iOS6/8/9/10/11/ _iOS11_Metal and the Theos iOS build enable ARC globally), so every record_stop / error-path teardown leaked four AVAssetWriter* objects and one GCD queue. Fix with two small helper macros gated on __has_feature(objc_arc): * RA_RELEASE_OBJ(field) -- [field release]; field = nil; (MRC) field = nil; (ARC) * RA_RELEASE_DQ(field) -- dispatch_release + nil on MRC (with a NULL guard since dispatch_release(NULL) is undefined; [nil release] is a safe no-op so the OBJ variant does not need one). Under ARC both macros expand to the same field-assignment-to-nil the code already used, so there is no semantic change on that build path. Under MRC the retain-count arithmetic is now correct. Also fixes the same leak on the 'canAddInput audio input' error path in avfoundation_record_init, where the freshly-allocated handle->audioInput was being dropped with = nil (the alloc+init from a handful of lines earlier leaked under MRC, then the assetWriter was used to record video-only). Thread-safety: unchanged. The release helper is only invoked from record_free / init error paths on the main thread after the encoding queue has drained; no concurrent readers of the ivars. Build matrix impact: this fixes the leak everywhere the file is compiled as MRC (top-level Makefile macOS builds and the RetroArch/_Metal/_OSX107/_iOS13 Xcode projects). The ARC builds (_iOS6/8/9/10/11/_iOS11_Metal + Theos) are unaffected because the ARC expansion is identical to the old code.
1 parent ebf6708 commit b327848

1 file changed

Lines changed: 36 additions & 6 deletions

File tree

record/drivers/record_avfoundation.m

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,36 @@
3333
#include "../../retroarch.h"
3434
#include "../../verbosity.h"
3535

36+
/* Release helpers that work identically under MRC and ARC.
37+
*
38+
* The ivars on record_avfoundation_t are Objective-C pointers
39+
* (AVAssetWriter*, AVAssetWriterInput*, etc. and dispatch_queue_t)
40+
* held in a plain C struct. Modern clang auto-qualifies ObjC struct
41+
* members as __strong under ARC, so assignment through them releases
42+
* the prior value automatically - plain `field = nil` is correct.
43+
*
44+
* Under MRC (RetroArch's top-level qb Makefile builds .m files without
45+
* -fobjc-arc, and so do the RetroArch/_Metal/_OSX107/_iOS13 Xcode
46+
* projects) no such auto-release happens, so `field = nil` would
47+
* overwrite a +1-retained pointer and leak the object. That is the
48+
* historical behaviour: every record_stop leaked four AVAssetWriter
49+
* objects and one GCD queue.
50+
*
51+
* Fix with the macros below: explicit release on the MRC path,
52+
* assignment-only on the ARC path. `[nil release]` is a safe no-op,
53+
* but `dispatch_release(NULL)` is undefined, so the DQ macro guards.
54+
*
55+
* The field expression is evaluated twice by the MRC expansion; all
56+
* five call sites use a direct struct-member lvalue with no aliasing
57+
* so this is safe. */
58+
#if __has_feature(objc_arc)
59+
# define RA_RELEASE_OBJ(_x) do { (_x) = nil; } while (0)
60+
# define RA_RELEASE_DQ(_x) do { (_x) = nil; } while (0)
61+
#else
62+
# define RA_RELEASE_OBJ(_x) do { [(_x) release]; (_x) = nil; } while (0)
63+
# define RA_RELEASE_DQ(_x) do { if (_x) { dispatch_release(_x); (_x) = nil; } } while (0)
64+
#endif
65+
3666
typedef struct record_avfoundation
3767
{
3868
AVAssetWriter *assetWriter;
@@ -140,11 +170,11 @@ static void avfoundation_release_handle(record_avfoundation_t *handle)
140170
if (!handle)
141171
return;
142172
scaler_ctx_gen_reset(&handle->scaler);
143-
handle->pixelBufferAdaptor = nil;
144-
handle->videoInput = nil;
145-
handle->audioInput = nil;
146-
handle->assetWriter = nil;
147-
handle->encodingQueue = nil;
173+
RA_RELEASE_OBJ(handle->pixelBufferAdaptor);
174+
RA_RELEASE_OBJ(handle->videoInput);
175+
RA_RELEASE_OBJ(handle->audioInput);
176+
RA_RELEASE_OBJ(handle->assetWriter);
177+
RA_RELEASE_DQ (handle->encodingQueue);
148178
free(handle);
149179
}
150180

@@ -343,7 +373,7 @@ static void avfoundation_release_handle(record_avfoundation_t *handle)
343373
else
344374
{
345375
RARCH_WARN("[AVFoundation] Cannot add audio input, continuing without audio\n");
346-
handle->audioInput = nil;
376+
RA_RELEASE_OBJ(handle->audioInput);
347377
}
348378
}
349379

0 commit comments

Comments
 (0)