Hot reload crashes the mac os desktop embedder

[zanderso]

When the first reload attempt has dirty libraries, the macOS desktop embedder silently crashes during reassemble. However, if the first reload has no dirty libraries, then subsequent reloads succeed.

@jonahwilliams @stuartmorgan

[jonahwilliams]

I was able to reproduce a crash, not sure if it was the crash. This is happening on the macOS desktop embedder when running in debug mode

flutter run
Building macOS application...                                           
../../third_party/dart/runtime/vm/object.cc: 3716: error: Unable to use class Library:'dart:ui' Class: TextRange which is not loaded yet.
version=2.7.0 (Wed Nov 6 22:44:33 2019 +0000) on "macos_x64"
thread=39179, isolate=main(0x7ff0528b1000)
  pc 0x000000010b818155 fp 0x00007000036f6350 dart::Profiler::DumpStackTrace(void*)
  pc 0x000000010b6b1a92 fp 0x00007000036f6430 dart::Assert::Fail(char const*, ...)
  pc 0x000000010b7a8c21 fp 0x00007000036f6460 dart::Class::EnsureDeclarationLoaded() const
  pc 0x000000010b6faafe fp 0x00007000036f64d0 dart::ClassFinalizer::FinalizeTypesInClass(dart::Class const&)
  pc 0x000000010b6fab6e fp 0x00007000036f6540 dart::ClassFinalizer::FinalizeTypesInClass(dart::Class const&)
  pc 0x000000010b6fa92e fp 0x00007000036f66c0 dart::ClassFinalizer::ProcessPendingClasses()
  pc 0x000000010bab78be fp 0x00007000036f66f0 dart::Api::CheckAndFinalizePendingClasses(dart::Thread*)
  pc 0x000000010bad24ce fp 0x00007000036f6830 Dart_FinalizeLoading
  pc 0x000000010b6a226e fp 0x00007000036f6870 flutter::DartIsolate::LoadKernel(std::__1::shared_ptr<fml::Mapping const>, bool)
  pc 0x000000010b6a2359 fp 0x00007000036f6920 flutter::DartIsolate::PrepareForRunningFromKernel(std::__1::shared_ptr<fml::Mapping const>, bool)
  pc 0x000000010b46b45e fp 0x00007000036f6960 flutter::KernelIsolateConfiguration::DoPrepareIsolate(flutter::DartIsolate&)
  pc 0x000000010b4643c8 fp 0x00007000036f6b40 flutter::Engine::PrepareAndLaunchIsolate(flutter::RunConfiguration)
  pc 0x000000010b463e93 fp 0x00007000036f6d60 flutter::Engine::Run(flutter::RunConfiguration)
  pc 0x000000010b4814cb fp 0x00007000036f6f00 std::__1::__function::__func<fml::internal::CopyableLambda<flutter::Shell::RunEngine(flutter::RunConfiguration, std::__1::function<void (flutter::Engine::RunStatus)>)::$_7>, std::__1::allocator<fml::internal::CopyableLambda<flutter::Shell::RunEngine(flutter::RunConfiguration, std::__1::function<void (flutter::Engine::RunStatus)>)::$_7> >, void ()>::operator()()
  pc 0x000000010b00a783 fp 0x00007000036f6f80 fml::MessageLoopImpl::FlushTasks(fml::FlushType)
  pc 0x000000010b01338c fp 0x00007000036f6fb0 fml::MessageLoopDarwin::OnTimerFire(__CFRunLoopTimer*, fml::MessageLoopDarwin*)
  pc 0x00007fff37d85060 fp 0x00007000036f6fc0 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
  pc 0x00007fff37d84c0c fp 0x00007000036f8070 __CFRunLoopDoTimer
  pc 0x00007fff37d84752 fp 0x00007000036f80d0 __CFRunLoopDoTimers
  pc 0x00007fff37d65962 fp 0x00007000036f8de0 __CFRunLoopRun
  pc 0x00007fff37d64ebe fp 0x00007000036f8e70 CFRunLoopRunSpecific
  pc 0x000000010b0135bd fp 0x00007000036f8eb0 fml::MessageLoopDarwin::Run()
  pc 0x000000010b00a624 fp 0x00007000036f8ed0 fml::MessageLoopImpl::DoRun()
  pc 0x000000010b0120f8 fp 0x00007000036f8f10 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, fml::Thread::Thread(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_0> >(void*)
  pc 0x00007fff63ed12eb fp 0x00007000036f8f30 _pthread_body
  pc 0x00007fff63ed4249 fp 0x00007000036f8f50 _pthread_start
  pc 0x00007fff63ed040d fp 0x00007000036f8f78 thread_start

I noticed that TextRange doesn't have an entrypoint annotation (should it?), but I did not think this mattered in debug mode.

cc @a-siva @alexmarkov

[jonahwilliams]

Actually, this isn't an entrypoint issue since the embedder isn't calling into text range.

[jonahwilliams]

Current theory from @alexmarkov is that I wasn't loading the core snapshots correctly, but since we were linking the platform in it didn't matter. Verifying that now

[jonahwilliams]

Forcing platform linking in debug mode issue casues the crash to disappear. I'm now looking into why we're not correctly supplying the core snapshots.

[jonahwilliams]

My read of the situation is:

1. The desktop targets will need to be updated to load isolate_snapshot_data and vm_snapshot_data in debug mode, using the core snapshots populated in the flutter assets directory.

2. The section in embedder.cc will need to be updated accept vm_snapshot_data and isolate_snapshot_data in non pre-recompiled mode. @chinmaygarde

In the meantime, we shouldn't keep the desktop targets broken though. I would propose that we force platform linking on desktop platforms for now

[stuartmorgan]

It sounds like there was a breaking change to the behavior of the embedding API? That's not supposed to happen, and would affect third-party projects that we don't control.

[jonahwilliams]

It wasn't a change to the embedder API, it was a change in how we built the initial kernel file that uncovered it. We never supplied the callbacks for isolate/vm data

EDIT: gramar spelling

[stuartmorgan]

I understand that the API didn't change, but it sounds like a change was made such that if someone were using the embedding API in the past, successfully, it will no longer work for them after an update unless they make changes to how they use it. That violates the compatibility intent of that API later as I have understood it.

[krisgiesing]

Just catching up on this. Can you elaborate on:

The desktop targets will need to be updated to load isolate_snapshot_data and vm_snapshot_data in debug mode

Is this something each desktop application will need to change? Or is it something that needs to be changed internally to Flutter?

[krisgiesing]

Also, is this crash reproducible in the desktop example app? Or does it require a more complex app to surface the issue?

[jonahwilliams]

We produce two artifacts ( a core snapshot) that contain parts of a precompiled dart-sdk for debug mode only. Android and iOS correctly load this artifact from the flutter assets_directory, but we never did the same for the desktop embedder. It happened to work, because we were previously compiling the entire dart-sdk into the initial kernel dill. To avoid this extra work, I disabled link-platform for debug mode fairly recently, which caused this particular macOS crash: since it doesn't have the core snapshots and doesn't have the linked platform there is no actual dart-sdk loaded into the program.

The workaround is to turn link-platform back on for desktop targets. The actual fix is to update the embedder to allow loading these snapshots in debug mode, and update the desktop shells to load them from flutter assets.

The change is internal to the desktop shells, applications won't need to do anything.

Also, is this crash reproducible in the desktop example app? Or does it require a more complex app to surface the issue?

This crash was trivially reproducible, it might be related to the other macOS crash but it needs more investigation. Matt and I are going to take a look on Friday.

[krisgiesing]

Can you link to the PR where link-platform was disabled? I'd like to correlate that change to when we started seeing problems on our end, as data around whether there is another problem lurking behind that one.

[jonahwilliams]

This would have broken in #44279 and the fix was landed in #44753

[krisgiesing]

OK. I don't think that change could account for all the issues we have been seeing, because it only landed 9 days ago, and we started seeing issues before that. Still, there was a separate app-specific fix that we landed today that might possibly account for the earlier failures.

[jonahwilliams]

We believe this is fixed with #44753. There might have been other issues, but as of last Friday neither me nor Matt could reproduce them.

I believe we'll need to increase the priority of #42281, this would have caught the issue during postsubmit.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.