-
-
Notifications
You must be signed in to change notification settings - Fork 40
fix: double-resume of checked continuation in ScreenTextExtractor.extractText(from:) crashes app on tiny OCR screenshots #698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import CoreGraphics | ||
| import XCTest | ||
| @testable import Cotabby | ||
|
|
||
| /// Direct tests for the Vision-backed OCR service boundary. | ||
| /// | ||
| /// Screenshot-context tests usually stub OCR so they can focus on orchestration. This file exists | ||
| /// separately because the crash happened inside `ScreenTextExtractor`'s callback-to-async bridge, | ||
| /// which only runs when the real Vision request path is exercised. | ||
| final class ScreenTextExtractorTests: XCTestCase { | ||
| func test_extractText_tinyImagesReturnNoRecognizedTextWithoutCrashing() async throws { | ||
| let extractor = ScreenTextExtractor() | ||
|
|
||
| for dimension in [1, 2] { | ||
| let image = try makeSolidImage(width: dimension, height: dimension) | ||
| await assertNoRecognizedText(from: image, extractor: extractor) | ||
| } | ||
|
Comment on lines
+14
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The guard threshold is Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| } | ||
|
|
||
| func test_extractText_blankNormalImageReturnsNoRecognizedText() async throws { | ||
| let image = try makeSolidImage(width: 128, height: 128) | ||
|
|
||
| await assertNoRecognizedText(from: image, extractor: ScreenTextExtractor()) | ||
| } | ||
|
|
||
| private func assertNoRecognizedText( | ||
| from image: CGImage, | ||
| extractor: ScreenTextExtractor, | ||
| file: StaticString = #filePath, | ||
| line: UInt = #line | ||
| ) async { | ||
| do { | ||
| _ = try await extractor.extractText(from: image) | ||
| XCTFail("Expected OCR to report no recognized text.", file: file, line: line) | ||
| } catch ScreenTextExtractionError.noRecognizedText { | ||
| // Expected: blank or degenerate screenshots should be treated as unavailable context. | ||
| } catch { | ||
| XCTFail("Expected noRecognizedText, got \(error).", file: file, line: line) | ||
| } | ||
| } | ||
|
|
||
| private func makeSolidImage(width: Int, height: Int) throws -> CGImage { | ||
| let colorSpace = CGColorSpaceCreateDeviceRGB() | ||
| let context = try XCTUnwrap( | ||
| CGContext( | ||
| data: nil, | ||
| width: width, | ||
| height: height, | ||
| bitsPerComponent: 8, | ||
| bytesPerRow: width * 4, | ||
| space: colorSpace, | ||
| bitmapInfo: CGImageAlphaInfo.premultipliedLast.rawValue | ||
| ) | ||
| ) | ||
| context.setFillColor(CGColor(gray: 1, alpha: 1)) | ||
| context.fill(CGRect(x: 0, y: 0, width: width, height: height)) | ||
| return try XCTUnwrap(context.makeImage()) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OCRContinuationResumermissing@unchecked SendableOCRContinuationResumeris captured by reference inside aDispatchQueue.global(qos:).asyncclosure, which requires its captured values to beSendablein Swift 5.7+ with-strict-concurrency. The class usesNSLockto protect all mutable state, so it is safe for concurrent access, but without declaring@unchecked Sendableit may produce a warning (or error under stricter project settings) like "capture of 'resumer' with non-sendable type 'OCRContinuationResumer' in an isolated closure". Adding the conformance makes the intent explicit and future-proofs the code.