diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 67f517ed..fcc1e0e5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,8 +11,9 @@ on: - master env: - # Use Xcode 15.2 or newer to support VisionOS - DEVELOPER_DIR: /Applications/Xcode_16.4.app + # Must exist on the current macos-latest runner image: + # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md + DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer jobs: debuggithub: @@ -28,9 +29,6 @@ jobs: analyze: name: Analyze runs-on: macos-latest - strategy: - matrix: - platform: ['iOS Simulator,name=iPhone 16,OS=18.5'] steps: - uses: actions/checkout@v2 - name: Analyze @@ -38,9 +36,6 @@ jobs: test: name: Test runs-on: macos-latest - strategy: - matrix: - platform: ['iOS Simulator,name=iPhone 16,OS=18.5'] steps: - uses: actions/checkout@v2 - name: Test diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 021f4de1..451e7e3a 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -7,8 +7,9 @@ on: required: true env: - # Use Xcode 15.2 or newer to support VisionOS - DEVELOPER_DIR: /Applications/Xcode_16.4.app + # Must exist on the current macos-latest runner image: + # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md + DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer jobs: create_release: diff --git a/Makefile b/Makefile index cf21392a..98f9c679 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ -PLATFORM="platform=iOS Simulator,name=iPhone 16,OS=18.5" -SDK="iphonesimulator18.5" +PLATFORM="platform=iOS Simulator,name=iPhone 17" +SDK="iphonesimulator" SHELL=/bin/bash -o pipefail XCODE_MAJOR_VERSION=$(shell xcodebuild -version | HEAD -n 1 | sed -E 's/Xcode ([0-9]+).*/\1/') IOS_EXAMPLE_PROJECT="Examples/Example-Xcode-SPM/Example-Xcode-SPM.xcodeproj" @@ -14,13 +14,13 @@ analyze: xcodebuild clean analyze -destination ${PLATFORM} -sdk ${SDK} -workspace PINRemoteImage.xcworkspace -scheme PINRemoteImage \ CODE_SIGNING_REQUIRED=NO \ CLANG_ANALYZER_OUTPUT=plist-html \ - CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcpretty + CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcbeautify if [[ -n `find $(shell pwd)/clang -name "*.html"` ]] ; then rm -rf `pwd`/clang; exit 1; fi rm -rf $(shell pwd)/clang test: xcodebuild clean test -destination ${PLATFORM} -sdk ${SDK} -workspace PINRemoteImage.xcworkspace -scheme PINRemoteImage \ - CODE_SIGNING_REQUIRED=NO | xcpretty + CODE_SIGNING_REQUIRED=NO | xcbeautify carthage: carthage update --no-use-binaries --no-build @@ -36,6 +36,6 @@ example: fi xcodebuild clean build -project ${IOS_EXAMPLE_PROJECT} -scheme ${EXAMPLE_SCHEME} -destination ${PLATFORM} -sdk ${SDK} \ ONLY_ACTIVE_ARCH=NO \ - CODE_SIGNING_REQUIRED=NO | xcpretty + CODE_SIGNING_REQUIRED=NO | xcbeautify all: carthage test cocoapods analyze spm example \ No newline at end of file diff --git a/Source/Classes/AnimatedImages/PINAPNGAnimatedImage.m b/Source/Classes/AnimatedImages/PINAPNGAnimatedImage.m index 0d39d833..55681211 100644 --- a/Source/Classes/AnimatedImages/PINAPNGAnimatedImage.m +++ b/Source/Classes/AnimatedImages/PINAPNGAnimatedImage.m @@ -19,6 +19,7 @@ #import #import +#import "PINRemoteLock.h" @interface PINAPNGAnimatedImage () { @@ -31,6 +32,7 @@ @interface PINAPNGAnimatedImage () size_t _loopCount; CFTimeInterval *_durations; NSError *_error; + PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource } @end @@ -40,6 +42,7 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData { if (self = [super init]) { _animatedImageData = animatedImageData; + _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINAPNGAnimatedImage decode lock"]; _imageSource = CGImageSourceCreateWithData((CFDataRef)animatedImageData, (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint: @@ -150,7 +153,16 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id)cacheProvider { - // I believe this is threadsafe as CGImageSource *seems* immutable… + // same hardening as PINGIFAnimatedImage — serialize + // all ImageIO calls on the shared source (including the status query, which + // advances parser state), and refuse affirmatively damaged frames. + [_decodeLock lock]; + CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index); + if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) { + [_decodeLock unlock]; + return NULL; + } + CGImageRef imageRef = CGImageSourceCreateImageAtIndex(_imageSource, index, @@ -161,7 +173,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id_animatedImage imageAtIndex:frameIndex cacheProvider:self]; PINLog(@"Generating: %lu", (unsigned long)frameIndex); - if (imageRef) { - __block PINImage *coverImage = nil; - __block PINAnimatedImageInfoReady coverImageReadyCallback = nil; - [self->_lock lockWithBlock:^{ + __block PINImage *coverImage = nil; + __block PINAnimatedImageInfoReady coverImageReadyCallback = nil; + [self->_lock lockWithBlock:^{ + if (imageRef) { [self->_frameCache setObject:(__bridge id _Nonnull)(imageRef) forKey:@(frameIndex)]; // Update the cover image @@ -373,26 +373,31 @@ - (void)_cacheWithFrameIndex:(NSUInteger)frameIndex coverImageReadyCallback = notifyCallback ? self->_coverImageReadyCallback : nil; coverImage = self->_coverImage; } + } else { + // a frame that fails to decode must release its + // render slot; leaving it in _cachedOrCachingFrames with _frameRenderCount + // held meant playbackReady never fired and the frame was never retried. + [self->_cachedOrCachingFrames removeIndex:frameIndex]; + } - self->_frameRenderCount--; - NSAssert(self->_frameRenderCount >= 0, @"playback ready is less than zero, something is wrong :("); + self->_frameRenderCount--; + NSAssert(self->_frameRenderCount >= 0, @"playback ready is less than zero, something is wrong :("); - PINLog(@"Frames left: %ld", (long)_frameRenderCount); + PINLog(@"Frames left: %ld", (long)_frameRenderCount); - dispatch_block_t notify = nil; - if (self->_frameRenderCount == 0 && self->_notifyOnReady) { - self->_notifyOnReady = NO; - if (self->_playbackReadyCallback) { - notify = self->_playbackReadyCallback; - [self->_operationQueue scheduleOperation:^{ - notify(); - }]; - } + dispatch_block_t notify = nil; + if (self->_frameRenderCount == 0 && self->_notifyOnReady) { + self->_notifyOnReady = NO; + if (self->_playbackReadyCallback) { + notify = self->_playbackReadyCallback; + [self->_operationQueue scheduleOperation:^{ + notify(); + }]; } - }]; - if (coverImageReadyCallback) { - coverImageReadyCallback(coverImage); } + }]; + if (coverImageReadyCallback) { + coverImageReadyCallback(coverImage); } } diff --git a/Source/Classes/AnimatedImages/PINGIFAnimatedImage.m b/Source/Classes/AnimatedImages/PINGIFAnimatedImage.m index e93763e8..0e789db5 100644 --- a/Source/Classes/AnimatedImages/PINGIFAnimatedImage.m +++ b/Source/Classes/AnimatedImages/PINGIFAnimatedImage.m @@ -17,6 +17,7 @@ #import #import +#import "PINRemoteLock.h" @interface PINGIFAnimatedImage () { @@ -29,6 +30,7 @@ @interface PINGIFAnimatedImage () size_t _loopCount; CFTimeInterval *_durations; NSError *_error; + PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource } @end @@ -38,6 +40,7 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData { if (self = [super init]) { _animatedImageData = animatedImageData; + _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINGIFAnimatedImage decode lock"]; _imageSource = CGImageSourceCreateWithData((CFDataRef)animatedImageData, (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint: @@ -148,7 +151,26 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id)cacheProvider { - // I believe this is threadsafe as CGImageSource *seems* immutable… + // serialize ALL ImageIO calls on the shared source. + // Despite the optimistic comment this replaced ("CGImageSource *seems* immutable"), + // CGImageSource is not safe for concurrent access, and PINCachedAnimatedImage + // reaches this concurrently from the caching queue, the init-time warmup block, + // and coverImage callers. The status query below also advances ImageIO parser + // state, so it must be inside the lock too. + [_decodeLock lock]; + + // Refuse frames whose data is affirmatively damaged. Frames are decoded lazily + // (kCGImageSourceShouldCache is false), so a damaged frame doesn't fail at + // creation — it crashes later inside CGContextDrawImage when CoreGraphics + // dereferences the failed decode. Only hard-failure statuses are rejected: + // kCGImageStatusIncomplete is what trailer-less-but-renderable GIFs (common in + // the wild) report, and those decode fine. + CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index); + if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) { + [_decodeLock unlock]; + return NULL; + } + CGImageRef imageRef = CGImageSourceCreateImageAtIndex(_imageSource, index, @@ -159,7 +181,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id #import +#import "PINRemoteLock.h" @interface PINWebPAnimatedImage () { @@ -32,6 +33,7 @@ @interface PINWebPAnimatedImage () size_t _loopCount; CFTimeInterval *_durations; NSError *_error; + PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource } @end @@ -41,7 +43,8 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData { if (self = [super init]) { _animatedImageData = animatedImageData; - + _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINWebPAnimatedImage decode lock"]; + _imageSource = CGImageSourceCreateWithData((CFDataRef)animatedImageData, (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint: @@ -152,7 +155,18 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id)cacheProvider { - // I believe this is threadsafe as CGImageSource *seems* immutable… + // same hardening as PINGIFAnimatedImage — serialize + // all ImageIO calls on the shared source (including the status query, which + // advances parser state), and refuse affirmatively damaged frames. CGImageSource + // is not safe for concurrent lazy decodes of the same source, despite the + // optimistic comment this replaced. + [_decodeLock lock]; + CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index); + if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) { + [_decodeLock unlock]; + return NULL; + } + CGImageRef imageRef = CGImageSourceCreateImageAtIndex(_imageSource, index, @@ -163,7 +177,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id