Fix Android SIGSEGV: don't free CGImage textures against a destroyed GL context - #429
Open
tanarie wants to merge 2 commits into
Open
Fix Android SIGSEGV: don't free CGImage textures against a destroyed GL context#429tanarie wants to merge 2 commits into
tanarie wants to merge 2 commits into
Conversation
tanarie
force-pushed
the
fix/free-cgimage-after-gl-context-loss
branch
from
July 30, 2026 10:01
44f1735 to
9d7c69f
Compare
…GL context On Android, backgrounding the app tears down the UIScreen and its GL context (UIScreen.main = nil -> UIScreen.deinit); resuming builds a new UIScreen/context. A CGImage created under the old context could then be freed after the new context exists: CGImage.deinit's guard only checked UIScreen.main != nil, which passes because a (new) screen exists, so GPU_FreeImage ran against the old image whose context_target is now freed memory -> SIGSEGV in renderer_GL_common.inl (GPU_FreeImage). Tag each CGImage with the UIScreen.contextGeneration it was created under and only free its GPU_Image when that generation still matches the live context, in both deinit and reloadFromSourceData(). A stale image's texture was already freed together with its context, so skipping the free leaks nothing.
tanarie
force-pushed
the
fix/free-cgimage-after-gl-context-loss
branch
from
July 30, 2026 10:03
9d7c69f to
757b772
Compare
tanarie
marked this pull request as ready for review
July 30, 2026 10:17
Makes the staleness check self-sufficient: `generation == contextGeneration` now means exactly "the context that owns this image is still the live one", rather than relying on `UIScreen.main` being nil for the whole teardown -> reinit window. If an old UIScreen's deinit were ever deferred past the new screen's creation, images from the new context would have looked live while GPU_Quit tore down their renderer. Also correct the claim about leaking: skipping GPU_FreeImage doesn't leak texture memory (that goes away with the context), but the two SDL_free calls at the end of FreeImage sit outside the GL branch, so the CPU-side GPU_Image/GPU_IMAGE_DATA structs do leak.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change: bug fix
Motivation
Fixes the ongoing Android crash:
https://app.bugsnag.com/flowkey/mobile-app/errors/69644a5e4ef4ae6af7fd0f05
SIGSEGVinrenderer_GL_common.inl:3923, insideGPU_FreeImage. Android only.Last 30 days: 108 events, 106 users; 198 events / 192 users all time. First seen
2026-01-12, still happening on 2.113.2. It correlates with the app going to the
background and coming back.
Root cause
On Android, backgrounding tears down the
UIScreenand its GL context(
UIScreen.main = nil→UIScreen.deinit→GPU_Quit()); resuming builds anew
UIScreen/context. ACGImagecreated under the old context canthen be freed after the new context exists, and
CGImage.deinit's guard onlychecked
UIScreen.main != nil— which passes, because a (new) screen exists. SoGPU_FreeImageran against an image whosecontext_targetis freed memory →crash. The guard asked "does a screen exist?", not "is the context that owns
this image still alive?".
Why it reliably lands after the resume:
deinitfrees viaTask { @MainActor },and Android pauses our render loop while backgrounded, so jobs queued during
teardown only drain on the first frame after resume — the stack trace shows the
free inside
nativeProcessEventsAndRender.Why sdl-gpu's own guard doesn't help: it compares
image->renderer == GPU_GetCurrentRenderer(), but the renderer isSDL_malloc'dand
SDL_free'd (renderer_GLES_2.c:32/:62), so theGPU_InitafterGPU_Quitusually gets the same address back and the comparison passesspuriously.
Fix
Tag each
CGImagewith theUIScreen.contextGenerationit was created under,and only call
GPU_FreeImagewhen that generation is still the current one —in both
deinitandreloadFromSourceData(). The counter is bumped whenever aGL context is created or destroyed, so "generation matches" means exactly
"the context that owns this image is still live", without depending on
UIScreen.mainbeing nil for the whole teardown→reinit window.Skipping the free doesn't leak texture memory (that goes with the context), but
it does leak the small
GPU_Image/GPU_IMAGE_DATAstructs —FreeImage's twoSDL_freecalls sit outside the GL branch. ~100 bytes per live image perbackground cycle. Freeing them properly needs a registry of live
CGImagestorn down before
GPU_Quit(); follow-up, not a blocker.Notes / testing
background, so it rarely triggers; the crash mostly hits Samsung/Mali devices.
count dropping after release.
Please check if the PR fulfills these requirements