Skip to content

[flutter_inappwebview] Fix SIGTRAP on TV app teardown - #1100

Open
seungsoo47 wants to merge 2 commits into
flutter-tizen:mainfrom
seungsoo47:flutter_inappwebview-tv-shutdown-fix2
Open

[flutter_inappwebview] Fix SIGTRAP on TV app teardown#1100
seungsoo47 wants to merge 2 commits into
flutter-tizen:mainfrom
seungsoo47:flutter_inappwebview-tv-shutdown-fix2

Conversation

@seungsoo47

Copy link
Copy Markdown
Contributor
  • Fix a SIGTRAP/SIGSEGV crash during app teardown on TV targets: ewk_init()/ewk_shutdown() were both commented out, but chromium-efl requires them to be called exactly once per process.
  • Add WebView::InitializeEngine()/ShutdownEngine(), called from the plugin's constructor/destructor. ShutdownEngine() waits (bounded, 2s) for all WebView instances to be disposed first, since ewk_shutdown() fatally CHECKs if any Ewk_View is still alive.

ewk_init() and ewk_shutdown() were both commented out, guessing that OS-level
EFL init and SetArguments() made ewk_init() unnecessary. In practice
chromium-efl needs ewk_init()/ewk_shutdown() called exactly once per process:
without ewk_init(), some engine state assumed initialized elsewhere is never
set up, and on TV targets this surfaces as a SIGTRAP/SIGSEGV during app
teardown.
Call ewk_init() once from the plugin's constructor and ewk_shutdown() once
from its destructor, via new WebView::InitializeEngine()/ShutdownEngine()
static helpers. ShutdownEngine() waits (bounded, 2s) for the live WebView
instance set to drain first, since ewk_shutdown() fatally CHECKs if any
Ewk_View is still alive; normal teardown order (platform views destroyed
before the plugin) already guarantees this, so the wait is defensive only.
Verified with 3 clean drive runs each on the TV emulator and a real TV
device, with no crash and no crash dumps.

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:4211a8498d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +234 to +238
if (g_get_monotonic_time() >= deadline) {
LOG_WARN(
"ShutdownEngine: WebView instance(s) still alive past the "
"deadline; calling ewk_shutdown() anyway.");
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not shut EWK down while views remain

When a platform view is still registered after this deadline—for example, if plugin teardown begins before its platform-view teardown can run—the loop merely logs and then reaches ewk_shutdown() with a live Ewk_View. The comment immediately above documents that this condition fatally CHECKs with SIGTRAP, so this fallback deterministically reproduces the crash it is intended to prevent; teardown must either dispose/flush the remaining views or skip shutdown rather than continuing.

Useful? React with 👍 / 👎.

cookie_channel)
: manager_channel_(std::move(manager_channel)),
cookie_channel_(std::move(cookie_channel)) {
WebView::InitializeEngine();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard the EWK lifecycle process-wide

When a Flutter engine is destroyed and recreated in the same process, each new plugin instance calls ewk_init() and the previous instance has already called ewk_shutdown(). This is a per-plugin lifecycle rather than the promised process-wide once-only lifecycle, and the existing EWK constraint described by this change says reinitialization after shutdown triggers SIGTRAP; use process-wide ownership/once semantics instead of pairing the calls with every plugin instance.

Useful? React with 👍 / 👎.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@seungsoo47