Skip to content

android: Only tear down the surface a destroy names - #23

Closed
Bombatomica64 wants to merge 1 commit into
longbridge:mainfrom
Bombatomica64:fix/host-surface-destroy-race
Closed

Bombatomica64 wants to merge 1 commit into
longbridge:mainfrom
Bombatomica64:fix/host-surface-destroy-race

Conversation

@Bombatomica64

Copy link
Copy Markdown

Problem

On the host-driven entry point (android::host), recreating the Activity in the same process (e.g. FLAG_ACTIVITY_CLEAR_TASK) could leave a black screen with a working app underneath. The new Activity's surfaceCreated can arrive before the old Activity's surfaceDestroyed. SurfaceDestroyed carried no identity, so the render thread terminated the window that was already attached to the new surface, and nothing re-attached it.

The wait in surface_destroyed() had a related problem: it waited on one shared SURFACE_RELEASED flag that a later surfaceCreated could reset, so overlapping destroy/create pairs could time out or return early.

Change

  • Command::SurfaceDestroyed(Option<usize>) carries the ANativeWindow being destroyed. The render thread calls term_window only if it matches CURRENT_SURFACE; otherwise it logs and ignores the stale destroy.
  • New pub fn surface_destroyed_for(window: &NativeWindow) for hosts that know which surface went away. surface_destroyed() keeps its signature and old meaning (it tears down whatever is attached), and its docs now point to the new function. The module docs recommend surface_destroyed_for.
  • The blocking wait uses request/handled counters (DESTROYS_REQUESTED / DESTROYS_HANDLED), so each caller waits for its own request. The 2 s bound is unchanged.

The android-activity path is untouched.

Testing

  • cargo fmt --all, cargo check, cargo ndk -t arm64-v8a check --all-features, and cargo test --lib (46 passed).
  • cargo test doc tests: platform_view and platform_view_element fail identically on main, so that failure is not from this change.
  • aarch64-apple-ios was not checked (Linux machine); the change is Android-only code.
  • On device: an Android test app on the host path (Android 13 emulator via Redroid) calls surface_destroyed_for from surfaceDestroyed. Before the change, an in-process recreation turned the screen black. After it, 5 consecutive in-process recreations all had their late destroys ignored, and rendering, touch and GPUI state stayed intact.

Related Kit report: longbridge/gpui-kit#3310

🤖 Generated with Claude Code

A SurfaceView's surfaceDestroyed for an old surface can arrive after the
replacement surface was already attached (e.g. on Activity recreation).
The host then terminated the new window and the app rendered black.

Commands now carry the destroyed ANativeWindow; the render thread ignores
a destroy that does not match the current surface. Add
surface_destroyed_for(window) for hosts that know which surface went
away; surface_destroyed() keeps its old meaning.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 07:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Bombatomica64

Copy link
Copy Markdown
Author

Sorry for the noise. I've combined this into a single PR, #24, so there's only one to review. Closing this one.

@Bombatomica64
Bombatomica64 deleted the fix/host-surface-destroy-race branch September 29, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants