mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 17:28:49 -10:00
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.
This commit is contained in:
1 parent
ebf670802f
commit
b32784877b
1 file changed
+36
-6
@@ -33,6 +33,36 @@
|
||||
#include "../../retroarch.h"
|
||||
#include "../../verbosity.h"
|
||||
|
||||
/* Release helpers that work identically under MRC and ARC.
|
||||
*
|
||||
* The ivars on record_avfoundation_t are Objective-C pointers
|
||||
* (AVAssetWriter*, AVAssetWriterInput*, etc. and dispatch_queue_t)
|
||||
* held in a plain C struct. Modern clang auto-qualifies ObjC struct
|
||||
* members as __strong under ARC, so assignment through them releases
|
||||
* the prior value automatically - plain `field = nil` is correct.
|
||||
*
|
||||
* Under MRC (RetroArch's top-level qb Makefile builds .m files without
|
||||
* -fobjc-arc, and so do the RetroArch/_Metal/_OSX107/_iOS13 Xcode
|
||||
* projects) no such auto-release happens, so `field = nil` would
|
||||
* overwrite a +1-retained pointer and leak the object. That is the
|
||||
* historical behaviour: every record_stop leaked four AVAssetWriter
|
||||
* objects and one GCD queue.
|
||||
*
|
||||
* Fix with the macros below: explicit release on the MRC path,
|
||||
* assignment-only on the ARC path. `[nil release]` is a safe no-op,
|
||||
* but `dispatch_release(NULL)` is undefined, so the DQ macro guards.
|
||||
*
|
||||
* The field expression is evaluated twice by the MRC expansion; all
|
||||
* five call sites use a direct struct-member lvalue with no aliasing
|
||||
* so this is safe. */
|
||||
#if __has_feature(objc_arc)
|
||||
# define RA_RELEASE_OBJ(_x) do { (_x) = nil; } while (0)
|
||||
# define RA_RELEASE_DQ(_x) do { (_x) = nil; } while (0)
|
||||
#else
|
||||
# define RA_RELEASE_OBJ(_x) do { [(_x) release]; (_x) = nil; } while (0)
|
||||
# define RA_RELEASE_DQ(_x) do { if (_x) { dispatch_release(_x); (_x) = nil; } } while (0)
|
||||
#endif
|
||||
|
||||
typedef struct record_avfoundation
|
||||
{
|
||||
AVAssetWriter *assetWriter;
|
||||
@@ -140,11 +170,11 @@ static void avfoundation_release_handle(record_avfoundation_t *handle)
|
||||
if (!handle)
|
||||
return;
|
||||
scaler_ctx_gen_reset(&handle->scaler);
|
||||
handle->pixelBufferAdaptor = nil;
|
||||
handle->videoInput = nil;
|
||||
handle->audioInput = nil;
|
||||
handle->assetWriter = nil;
|
||||
handle->encodingQueue = nil;
|
||||
RA_RELEASE_OBJ(handle->pixelBufferAdaptor);
|
||||
RA_RELEASE_OBJ(handle->videoInput);
|
||||
RA_RELEASE_OBJ(handle->audioInput);
|
||||
RA_RELEASE_OBJ(handle->assetWriter);
|
||||
RA_RELEASE_DQ (handle->encodingQueue);
|
||||
free(handle);
|
||||
}
|
||||
|
||||
@@ -343,7 +373,7 @@ static void *avfoundation_record_init(const struct record_params *params)
|
||||
else
|
||||
{
|
||||
RARCH_WARN("[AVFoundation] Cannot add audio input, continuing without audio\n");
|
||||
handle->audioInput = nil;
|
||||
RA_RELEASE_OBJ(handle->audioInput);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user