Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 584bad1fd8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SetUpTizenVideoPlayerApiFfi(this); | ||
| SetUpVideoPlayerEventsVideoEventsFfi(this); |
There was a problem hiding this comment.
With separate Dart/UI and platform threads, omitting the dispatcher makes the generated FFI wrappers execute inline on the Dart thread, while ScheduleSendPendingEvents() still delivers events through the captured GLib context. Consequently, OnCancel() can call ClearEventSink() concurrently with SendPendingEvents() invoking event_sink_->Success(), freeing the sink during dispatch. An ASan harness using the extracted player methods and generated sink reproduces this heap-use-after-free. The previous EventChannel handlers were serialized on the platform thread. Supply a main-context dispatcher for both registrations, or synchronize sink replacement/cancellation with event delivery.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
| const auto& codec = PigeonFfiGetCodec(); | ||
| on_event_(sink_id_, PigeonFfiEncodeMessage(codec, ::flutter::EncodableValue(event))); |
There was a problem hiding this comment.
When hot restart occurs with a live player, the native sink survives but the Dart isolate owning its NativeCallable.listener pointers is destroyed. Old players are only cleared when the next isolate calls VideoPlayerTizenPlugin::Initialize(), so a completion or error before that cleanup invokes an expired callback here and can terminate the process. Unlike the previous messenger-backed delivery, these raw callback pointers cannot safely outlive their originating isolate. Quiesce and detach native callbacks before isolate teardown, or use delivery that safely rejects notifications to dead isolates rather than relying on the next initialization.
Useful? React with 👍 / 👎.
Co-Authored-By: Cline SR
584bad1 to
530b82e
Compare
Main changes:
BasicMessageChannel.EventChannelis replaced by a Pigeon FFI event stream (videoEvents), with the texture id passed as the stream instance name.