Skip to content
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

Fix segfault on isolate shutdown #163

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Conversation

alexlapa
Copy link
Contributor

@alexlapa alexlapa commented Apr 11, 2024

medea#225

Synopsis

Thread 4 Crashed:: io.flutter.ui
0   FlutterMacOS                           0x10a7a501c Dart_HandleFromPersistent + 312
1   libmedea_jason.dylib                   0x102baa88c $medea_jason..platform..dart..transport..WebSocketRpcTransport$u20$as$u20$medea_jason..platform..transport..RpcTransport::send::h4e79a68bb1cd2ffd + 344
2   libmedea_jason.dylib                   0x102b22f04 medea_jason::rpc::websocket::client::WebSocketRpcClient::send_command::h4a8a35a20f623e0a + 168
3   libmedea_jason.dylib                   0x102b5c898 $medea_jason..rpc..rpc_session..WebSocketRpcSession$u20$as$u20$medea_jason..rpc..rpc_session..RpcSession::close_with_reason::h926fa61b82b07dd8 + 416
4   libmedea_jason.dylib                   0x102b9ebd4 core::ptr::drop_in_place$LT$medea_jason..room..InnerRoom::ha131d02d86c86ed5 + 108
5   libmedea_jason.dylib                   0x102b9c74c core::ptr::drop_in_place$LT$core..cell..RefCell$LT$medea_jason..jason..Inner::h0f47d0718bb2b058 + 172
6   libmedea_jason.dylib                   0x102b368dc alloc::sync::Arc$LT$T$C$A::drop_slow::h4bc275aa9831024a + 44
7   FlutterMacOS                           0x10a472908 dart::NativeFinalizer::RunCallback(dart::FinalizerEntry const&, char const*) const + 176
8   FlutterMacOS                           0x10a3cc2b0 dart::Isolate::RunAndCleanupFinalizersOnShutdown() + 576
9   FlutterMacOS                           0x10a3cc79c dart::Isolate::Shutdown() + 216
10  FlutterMacOS                           0x10a7a6dd8 Dart_ShutdownIsolate + 224
11  FlutterMacOS                           0x10a2fb628 flutter::DartIsolate::Shutdown() + 100
12  FlutterMacOS                           0x10a308ee8 flutter::RuntimeController::~RuntimeController() + 152
13  FlutterMacOS                           0x10a3090cc flutter::RuntimeController::~RuntimeController() + 32
14  FlutterMacOS                           0x10a235d80 flutter::Engine::~Engine() + 348

Dart-side websocket is reclaimed by flutter before Room, so when Room tries to send LeaveRoom command in Drop implementation application segfaults. This happens if application is closed during call.

Solution

Integrators should use AppLifecycleListener to explicitly dispose all medea-jason objects before Dart_ShutdownIsolate is called.

But anyway we dont really want to segfault if this recommendation is not followed, so this PR wraps Room's Drop implementation in platfrom::spawn, this will prevent segfault since spawned future will not be completed if isolate is currently shutting down.

Checklist

  • Created PR:
    • In draft mode
    • Name contains issue reference
    • Has type and k:: labels applied
    • Has assignee
  • Before review:
    • Documentation is updated (if required)
    • Tests are updated (if required)
    • Changes conform code style
    • CHANGELOG entry is added (if required)
    • FCM (final commit message) is posted or updated
    • Draft mode is removed
  • Review is completed and changes are approved
    • FCM (final commit message) is approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • All temporary labels are removed

@alexlapa alexlapa added enhancement Improvement of existing features or bugfix k::refactor Refactoring, technical debt elimination and other improvements of existing code base platform::android Specific to Android platform platform::ios Specific to iOS platform platform::windows Specific to Windows platform platform::linux Specific to Linux platform labels Apr 11, 2024
@alexlapa alexlapa self-assigned this Apr 11, 2024
@alexlapa
Copy link
Contributor Author

FCM:

Fix segfault on Dart isolate shutdown (#163)

@alexlapa alexlapa marked this pull request as ready for review April 12, 2024 08:24
@alexlapa alexlapa requested a review from evdokimovs April 12, 2024 08:24
@evdokimovs evdokimovs requested a review from tyranron April 12, 2024 10:16
@tyranron tyranron merged commit 975b20f into master Apr 12, 2024
9 of 42 checks passed
@tyranron tyranron deleted the fix-segfault-on-isolate-shutdown branch April 12, 2024 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::refactor Refactoring, technical debt elimination and other improvements of existing code base platform::android Specific to Android platform platform::ios Specific to iOS platform platform::linux Specific to Linux platform platform::windows Specific to Windows platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants