-
Notifications
You must be signed in to change notification settings - Fork 6k
Avoid crashing and display error if the process cannot be prepared for JIT mode Dart VM. #20980
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,36 +19,49 @@ | |
| // - go/decommissioning-dbc | ||
| // - go/decommissioning-dbc-engine | ||
| // - go/decommissioning-dbc-tools | ||
| #include "flutter/common/settings.h" | ||
| #include "flutter/fml/build_config.h" // For OS_IOS. | ||
|
|
||
| #if OS_IOS && (FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG) | ||
| #include "flutter/runtime/ptrace_check.h" | ||
|
|
||
| #if TRACING_CHECKS_NECESSARY | ||
|
|
||
| // These headers should only be needed in debug mode. | ||
| #include <sys/sysctl.h> | ||
| #include <sys/types.h> | ||
|
|
||
| #include <mutex> | ||
|
|
||
| #include "flutter/fml/build_config.h" | ||
|
|
||
| // Being extra careful and adding additional landmines that will prevent | ||
| // compilation of this TU in an incorrect runtime mode. | ||
| static_assert(OS_IOS, "This translation unit is iOS specific."); | ||
| static_assert(FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG, | ||
| "This translation unit must only be compiled in the debug " | ||
| "runtime mode as it " | ||
| "contains private API usage."); | ||
|
|
||
| #define PT_TRACE_ME 0 | ||
| #define PT_SIGEXC 12 | ||
| extern "C" int ptrace(int request, pid_t pid, caddr_t addr, int data); | ||
|
|
||
| static bool DebuggedIOS(const flutter::Settings& vm_settings) { | ||
| namespace flutter { | ||
|
|
||
| static bool IsLaunchedByFlutterCLI(const Settings& vm_settings) { | ||
| // Only the Flutter CLI passes "--enable-checked-mode". Therefore, if the flag | ||
| // is present, we have been launched by "ios-deploy" via "debugserver". | ||
| // | ||
| // We choose this flag because it is always passed to launch debug builds. | ||
| if (vm_settings.enable_checked_mode) { | ||
| return true; | ||
| } | ||
| return vm_settings.enable_checked_mode; | ||
| } | ||
|
|
||
| static bool IsLaunchedByXcode() { | ||
| // Use "sysctl()" to check if we're currently being debugged (e.g. by Xcode). | ||
| // We could also check "getppid() != 1" (launchd), but this is more direct. | ||
| const pid_t self = getpid(); | ||
| int mib[5] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, self, 0}; | ||
|
|
||
| auto proc = std::make_unique<struct kinfo_proc>(); | ||
| size_t proc_size = sizeof(struct kinfo_proc); | ||
| if (sysctl(mib, 4, proc.get(), &proc_size, nullptr, 0) < 0) { | ||
| if (::sysctl(mib, 4, proc.get(), &proc_size, nullptr, 0) < 0) { | ||
| FML_LOG(ERROR) << "Could not execute sysctl() to get current process info: " | ||
| << strerror(errno); | ||
| return false; | ||
|
|
@@ -57,18 +70,16 @@ static bool DebuggedIOS(const flutter::Settings& vm_settings) { | |
| return proc->kp_proc.p_flag & P_TRACED; | ||
| } | ||
|
|
||
| void EnsureDebuggedIOS(const flutter::Settings& vm_settings) { | ||
| if (DebuggedIOS(vm_settings)) { | ||
| return; | ||
| } | ||
|
|
||
| if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1) { | ||
| static bool EnableTracingManually(const Settings& vm_settings) { | ||
| if (::ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1) { | ||
| FML_LOG(ERROR) << "Could not call ptrace(PT_TRACE_ME): " << strerror(errno); | ||
| // No use trying PT_SIGEXC -- it's only needed if PT_TRACE_ME succeeds. | ||
| return; | ||
| return false; | ||
| } | ||
| if (ptrace(PT_SIGEXC, 0, nullptr, 0) == -1) { | ||
|
|
||
| if (::ptrace(PT_SIGEXC, 0, nullptr, 0) == -1) { | ||
| FML_LOG(ERROR) << "Could not call ptrace(PT_SIGEXC): " << strerror(errno); | ||
| return false; | ||
| } | ||
|
|
||
| // The previous operation causes this process to not be reaped after it | ||
|
|
@@ -78,11 +89,12 @@ void EnsureDebuggedIOS(const flutter::Settings& vm_settings) { | |
| size_t maxproc = 0; | ||
| size_t maxproc_size = sizeof(size_t); | ||
| const int sysctl_result = | ||
| sysctlbyname("kern.maxproc", &maxproc, &maxproc_size, nullptr, 0); | ||
| ::sysctlbyname("kern.maxproc", &maxproc, &maxproc_size, nullptr, 0); | ||
| if (sysctl_result < 0) { | ||
| FML_LOG(ERROR) | ||
| << "Could not execute sysctl() to determine process count limit: " | ||
| << strerror(errno); | ||
| return false; | ||
| } | ||
|
|
||
| const char* warning = | ||
|
|
@@ -98,6 +110,39 @@ void EnsureDebuggedIOS(const flutter::Settings& vm_settings) { | |
| { | ||
| FML_LOG(ERROR) << warning; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| static bool EnableTracingIfNecessaryOnce(const Settings& vm_settings) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the name have once in it? Nothing here enforces the once-ness here it seems.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind, seems like the name is reflects the expectation rather than the implementation... Seems a bit strange but I don't have a strong opinion. If you expect something, you should probably assert it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was following the usual C convention of of calling a method that is supposed to be called when a mutex is held with the |
||
| if (IsLaunchedByFlutterCLI(vm_settings)) { | ||
| return true; | ||
| } | ||
|
|
||
| if (IsLaunchedByXcode()) { | ||
| return true; | ||
| } | ||
|
|
||
| return EnableTracingManually(vm_settings); | ||
| } | ||
|
|
||
| #endif // OS_IOS && (FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG) | ||
| static TracingResult sTracingResult = TracingResult::kNotAttempted; | ||
|
|
||
| bool EnableTracingIfNecessaryImpl(const Settings& vm_settings) { | ||
| static std::once_flag tracing_flag; | ||
|
|
||
| std::call_once(tracing_flag, [&vm_settings]() { | ||
| sTracingResult = EnableTracingIfNecessaryOnce(vm_settings) | ||
| ? TracingResult::kEnabled | ||
| : TracingResult::kDisabled; | ||
| }); | ||
| return sTracingResult != TracingResult::kDisabled; | ||
| } | ||
|
|
||
| TracingResult GetTracingResultImpl() { | ||
| return sTracingResult; | ||
| } | ||
|
|
||
| } // namespace flutter | ||
|
|
||
| #endif // TRACING_CHECKS_NECESSARY | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef FLUTTER_RUNTIME_PTRACE_CHECK_H_ | ||
| #define FLUTTER_RUNTIME_PTRACE_CHECK_H_ | ||
|
|
||
| #include "flutter/common/settings.h" | ||
| #include "flutter/fml/build_config.h" | ||
|
|
||
| namespace flutter { | ||
|
|
||
| #define TRACING_CHECKS_NECESSARY \ | ||
| OS_IOS && !TARGET_OS_SIMULATOR && \ | ||
| (FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG) | ||
|
|
||
| enum class TracingResult { | ||
| kNotAttempted, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docs?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What part would you like clarified? |
||
| kEnabled, | ||
| kNotNecessary = kEnabled, | ||
| kDisabled, | ||
| }; | ||
|
|
||
| #if TRACING_CHECKS_NECESSARY | ||
| bool EnableTracingIfNecessaryImpl(const Settings& vm_settings); | ||
| TracingResult GetTracingResultImpl(); | ||
| #endif // TRACING_CHECKS_NECESSARY | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| /// @brief Enables tracing in the process so that JIT mode VMs may be | ||
| /// launched. Explicitly enabling tracing is not required on all | ||
| /// platforms. On platforms where it is not required, calling this | ||
| /// method will return true. If tracing is required but cannot be | ||
| /// enabled, it is the responsibility of the caller to display the | ||
| /// appropriate error message to the user as subsequent attempts to | ||
| /// launch the VM in JIT mode will cause process termination. | ||
| /// | ||
| /// This method may be called multiple times and will return the | ||
| /// same result. There are no threading restrictions. | ||
| /// | ||
| /// @param[in] vm_settings The settings used to launch the VM. | ||
| /// | ||
| /// @return If tracing was enabled. | ||
| /// | ||
| inline bool EnableTracingIfNecessary(const Settings& vm_settings) { | ||
| #if TRACING_CHECKS_NECESSARY | ||
| return EnableTracingIfNecessaryImpl(vm_settings); | ||
| #else // TRACING_CHECKS_NECESSARY | ||
| return true; | ||
| #endif // TRACING_CHECKS_NECESSARY | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| /// @brief Returns if a tracing check has been performed and its result. To | ||
| /// enable tracing, the Settings object used to launch the VM is | ||
| /// required. Components may want to display messages based on the | ||
| /// result of a previous tracing check without actually having the | ||
| /// settings object. This accessor can be used instead. | ||
| /// | ||
| /// @return The tracing result. | ||
| /// | ||
| inline TracingResult GetTracingResult() { | ||
| #if TRACING_CHECKS_NECESSARY | ||
| return GetTracingResultImpl(); | ||
| #else // TRACING_CHECKS_NECESSARY | ||
| return TracingResult::kNotNecessary; | ||
| #endif // TRACING_CHECKS_NECESSARY | ||
| } | ||
|
|
||
| } // namespace flutter | ||
|
|
||
| #endif // FLUTTER_RUNTIME_PTRACE_CHECK_H_ | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| #include "flutter/fml/message_loop.h" | ||
| #include "flutter/fml/platform/darwin/platform_version.h" | ||
| #include "flutter/fml/trace_event.h" | ||
| #include "flutter/runtime/ptrace_check.h" | ||
| #include "flutter/shell/common/engine.h" | ||
| #include "flutter/shell/common/platform_view.h" | ||
| #include "flutter/shell/common/shell.h" | ||
|
|
@@ -114,6 +115,16 @@ - (instancetype)initWithName:(NSString*)labelPrefix | |
| else | ||
| _dartProject.reset([project retain]); | ||
|
|
||
| if (!EnableTracingIfNecessary([_dartProject.get() settings])) { | ||
| NSLog( | ||
| @"Cannot create a FlutterEngine instance in debug mode without Flutter tooling or " | ||
| @"Xcode.\n\nTo launch in debug mode in iOS 14+, run flutter run from Flutter tools, run " | ||
| @"from an IDE with a Flutter IDE plugin or run the iOS project from Xcode.\nAlternatively " | ||
| @"profile and release mode apps can be launched from the home screen."); | ||
| [self release]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we assert here or do something more eye-catching than a console message?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we assert, we will take down the process. We don't want to do that. Instead, we want to display an error message in existing applications.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's implicit but we should describe the solution explicitly.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack. |
||
| return nil; | ||
| } | ||
|
|
||
| _pluginPublications = [NSMutableDictionary new]; | ||
| _registrars = [[NSMutableDictionary alloc] init]; | ||
| _platformViewsController.reset(new flutter::FlutterPlatformViewsController()); | ||
|
|
@@ -514,6 +525,7 @@ - (BOOL)createShell:(NSString*)entrypoint | |
| _threadHost.ui_thread->GetTaskRunner(), // ui | ||
| _threadHost.io_thread->GetTaskRunner() // io | ||
| ); | ||
|
|
||
| // Create the shell. This is a blocking operation. | ||
| _shell = flutter::Shell::Create(std::move(task_runners), // task runners | ||
| std::move(platformData), // window data | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some code comments for readers (e.g. these 2 ifs are expected to fail on iOS 14, the rest should let people jit on iOS 13 etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can expect that. It's a private API and all implementation detail that can change at anytime without warning. I'd rather just be resilient to errors.