From 24c7826e89ed50029826016f369df78e7b806e88 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 16 Jun 2023 10:45:13 -0700 Subject: [PATCH 1/7] Print a warning when a message channel is used on the wrong thread. --- ci/licenses_golden/licenses_flutter | 2 + fml/BUILD.gn | 1 + fml/logging.cc | 102 ++++++++++++++++------------ fml/logging.h | 3 + fml/memory/thread_checker.cc | 11 +++ fml/memory/thread_checker.h | 16 ++++- shell/common/shell.cc | 18 ++++- shell/common/shell.h | 5 ++ shell/common/shell_test.cc | 5 ++ shell/common/shell_test.h | 3 + shell/common/shell_unittests.cc | 30 ++++++++ 11 files changed, 150 insertions(+), 46 deletions(-) create mode 100644 fml/memory/thread_checker.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 09c825e26c410..2757634435851 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -899,6 +899,7 @@ ORIGIN: ../../../flutter/fml/memory/ref_ptr.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/ref_ptr_internal.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/task_runner_checker.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/task_runner_checker.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/fml/memory/thread_checker.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/thread_checker.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/weak_ptr.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/fml/memory/weak_ptr_internal.cc + ../../../flutter/LICENSE @@ -3576,6 +3577,7 @@ FILE: ../../../flutter/fml/memory/ref_ptr.h FILE: ../../../flutter/fml/memory/ref_ptr_internal.h FILE: ../../../flutter/fml/memory/task_runner_checker.cc FILE: ../../../flutter/fml/memory/task_runner_checker.h +FILE: ../../../flutter/fml/memory/thread_checker.cc FILE: ../../../flutter/fml/memory/thread_checker.h FILE: ../../../flutter/fml/memory/weak_ptr.h FILE: ../../../flutter/fml/memory/weak_ptr_internal.cc diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 5ccc3919b91d0..7906c32968b4c 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -47,6 +47,7 @@ source_set("fml") { "memory/ref_ptr_internal.h", "memory/task_runner_checker.cc", "memory/task_runner_checker.h", + "memory/thread_checker.cc", "memory/thread_checker.h", "memory/weak_ptr.h", "memory/weak_ptr_internal.cc", diff --git a/fml/logging.cc b/fml/logging.cc index 61b4b5cf27858..b498483455c8d 100644 --- a/fml/logging.cc +++ b/fml/logging.cc @@ -72,61 +72,75 @@ LogMessage::LogMessage(LogSeverity severity, } } +// static +std::shared_ptr LogMessage::test_stream_ = nullptr; + +// static +void LogMessage::CaptureNextLog( + const std::shared_ptr& stream) { + test_stream_ = stream; +} + LogMessage::~LogMessage() { #if !defined(OS_FUCHSIA) stream_ << std::endl; #endif + if (test_stream_) { + *test_stream_ << stream_.str(); + test_stream_.reset(); + } else { #if defined(FML_OS_ANDROID) - android_LogPriority priority = - (severity_ < 0) ? ANDROID_LOG_VERBOSE : ANDROID_LOG_UNKNOWN; - switch (severity_) { - case LOG_INFO: - priority = ANDROID_LOG_INFO; - break; - case LOG_WARNING: - priority = ANDROID_LOG_WARN; - break; - case LOG_ERROR: - priority = ANDROID_LOG_ERROR; - break; - case LOG_FATAL: - priority = ANDROID_LOG_FATAL; - break; - } - __android_log_write(priority, "flutter", stream_.str().c_str()); + android_LogPriority priority = + (severity_ < 0) ? ANDROID_LOG_VERBOSE : ANDROID_LOG_UNKNOWN; + switch (severity_) { + case LOG_INFO: + priority = ANDROID_LOG_INFO; + break; + case LOG_WARNING: + priority = ANDROID_LOG_WARN; + break; + case LOG_ERROR: + priority = ANDROID_LOG_ERROR; + break; + case LOG_FATAL: + priority = ANDROID_LOG_FATAL; + break; + } + __android_log_write(priority, "flutter", stream_.str().c_str()); #elif defined(FML_OS_IOS) - syslog(LOG_ALERT, "%s", stream_.str().c_str()); + syslog(LOG_ALERT, "%s", stream_.str().c_str()); #elif defined(OS_FUCHSIA) - fx_log_severity_t fx_severity; - switch (severity_) { - case LOG_INFO: - fx_severity = FX_LOG_INFO; - break; - case LOG_WARNING: - fx_severity = FX_LOG_WARNING; - break; - case LOG_ERROR: - fx_severity = FX_LOG_ERROR; - break; - case LOG_FATAL: - fx_severity = FX_LOG_FATAL; - break; - default: - if (severity_ < 0) { - fx_severity = fx_log_severity_from_verbosity(-severity_); - } else { - // Unknown severity. Use INFO. + fx_log_severity_t fx_severity; + switch (severity_) { + case LOG_INFO: fx_severity = FX_LOG_INFO; - } - } - fx_logger_log_with_source(fx_log_get_logger(), fx_severity, nullptr, file_, - line_, stream_.str().c_str()); + break; + case LOG_WARNING: + fx_severity = FX_LOG_WARNING; + break; + case LOG_ERROR: + fx_severity = FX_LOG_ERROR; + break; + case LOG_FATAL: + fx_severity = FX_LOG_FATAL; + break; + default: + if (severity_ < 0) { + fx_severity = fx_log_severity_from_verbosity(-severity_); + } else { + // Unknown severity. Use INFO. + fx_severity = FX_LOG_INFO; + } + } + fx_logger_log_with_source(fx_log_get_logger(), fx_severity, nullptr, file_, + line_, stream_.str().c_str()); #else - // Don't use std::cerr here, because it may not be initialized properly yet. - fprintf(stderr, "%s", stream_.str().c_str()); - fflush(stderr); + // Don't use std::cerr here, because it may not be initialized properly yet. + fprintf(stderr, "%s", stream_.str().c_str()); + fflush(stderr); #endif + } if (severity_ >= LOG_FATAL) { KillProcess(); diff --git a/fml/logging.h b/fml/logging.h index 0b732ee818a0a..9596560ba857f 100644 --- a/fml/logging.h +++ b/fml/logging.h @@ -27,7 +27,10 @@ class LogMessage { std::ostream& stream() { return stream_; } + static void CaptureNextLog(const std::shared_ptr& stream); + private: + static std::shared_ptr test_stream_; std::ostringstream stream_; const LogSeverity severity_; const char* file_; diff --git a/fml/memory/thread_checker.cc b/fml/memory/thread_checker.cc new file mode 100644 index 0000000000000..77bbd3d6b9532 --- /dev/null +++ b/fml/memory/thread_checker.cc @@ -0,0 +1,11 @@ +// 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. + +#include "flutter/fml/memory/thread_checker.h" + +namespace fml { + +bool ThreadChecker::disable_next_ = false; + +} diff --git a/fml/memory/thread_checker.h b/fml/memory/thread_checker.h index a5aed0d2b7c0c..f0af15b06c638 100644 --- a/fml/memory/thread_checker.h +++ b/fml/memory/thread_checker.h @@ -34,7 +34,14 @@ class ThreadChecker final { ThreadChecker() : self_(GetCurrentThreadId()) {} ~ThreadChecker() {} - bool IsCreationThreadCurrent() const { return GetCurrentThreadId() == self_; } + bool IsCreationThreadCurrent() const { + bool result = GetCurrentThreadId() == self_; + if (!result && disable_next_) { + disable_next_ = false; + return true; + } + return result; + } private: DWORD self_; @@ -48,6 +55,10 @@ class ThreadChecker final { bool IsCreationThreadCurrent() const { pthread_t current_thread = pthread_self(); bool is_creation_thread_current = !!pthread_equal(current_thread, self_); + if (disable_next_ && !is_creation_thread_current) { + disable_next_ = false; + return true; + } #ifdef __APPLE__ // TODO(https://github.com/flutter/flutter/issues/45272): Implement for // other platforms. @@ -67,7 +78,10 @@ class ThreadChecker final { return is_creation_thread_current; } + static void DisableNextThreadCheckFailure() { disable_next_ = true; } + private: + static bool disable_next_; pthread_t self_; #endif }; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 7487147f57783..34dd9ef3ac0a4 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -992,11 +992,27 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { } } +std::set Shell::misbehaving_message_channels_ = {}; + // |PlatformView::Delegate| void Shell::OnPlatformViewDispatchPlatformMessage( std::unique_ptr message) { FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { + auto inserted = misbehaving_message_channels_.insert(message->channel()); + if (inserted.second) { + FML_LOG(ERROR) + << "The '" << message->channel() + << "' channel sent a message from native to Flutter on a " + "non-platform thread. Platform channel messages must be sent on " + "the platform thread. Failure to do so may result in data loss or " + "crashes, and must be fixed in the plugin or application code " + "creating that channel.\n" + "See https://docs.flutter.dev/platform-integration/" + "platform-channels#channels-and-platform-threading for more " + "information."; + } + } // The static leak checker gets confused by the use of fml::MakeCopyable. // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) diff --git a/shell/common/shell.h b/shell/common/shell.h index 6f6360625f412..8bee63d31b06c 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -412,6 +412,11 @@ class Shell final : public PlatformView::Delegate, std::function; + /// A collection of message channels (by name) that have sent at least one + /// message from a non-platform thread. Used to prevent printing the error log + /// more than once per channel, as a badly behaving plugin may send multiple + /// messages per second indefinitely. + static std::set misbehaving_message_channels_; const TaskRunners task_runners_; const fml::RefPtr parent_raster_thread_merger_; std::shared_ptr diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index f5af475863e33..5e5d38d2b2b94 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -25,6 +25,11 @@ ShellTest::ShellTest() ThreadHost::Type::Platform | ThreadHost::Type::IO | ThreadHost::Type::UI | ThreadHost::Type::RASTER) {} +void ShellTest::SendPlatformMessage(Shell* shell, + std::unique_ptr message) { + shell->OnPlatformViewDispatchPlatformMessage(std::move(message)); +} + void ShellTest::SendEnginePlatformMessage( Shell* shell, std::unique_ptr message) { diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index c60c1c10e1525..1fd588e3b8447 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -56,6 +56,9 @@ class ShellTest : public FixtureTest { fml::TimePoint GetLatestFrameTargetTime(Shell* shell) const; + void SendPlatformMessage(Shell* shell, + std::unique_ptr message); + void SendEnginePlatformMessage(Shell* shell, std::unique_ptr message); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 06a05399e8eda..f22edee0dcd07 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #define FML_USED_ON_EMBEDDER #include @@ -4231,6 +4232,35 @@ TEST_F(ShellTest, NotifyDestroyed) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); } +TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { + Settings settings = CreateSettingsForFixture(); + ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".", + ThreadHost::Type::Platform); + auto task_runner = thread_host.platform_thread->GetTaskRunner(); + TaskRunners task_runners("test", task_runner, task_runner, task_runner, + task_runner); + auto shell = CreateShell(settings, task_runners); + + auto stream = std::make_shared(); + fml::LogMessage::CaptureNextLog(stream); + + // The next call will result in a thread checker violation. + fml::ThreadChecker::DisableNextThreadCheckFailure(); + SendPlatformMessage(shell.get(), std::make_unique( + "com.test.plugin", nullptr)); + + EXPECT_THAT(stream->str(), + ::testing::EndsWith( + "The 'com.test.plugin' channel sent a message from native to " + "Flutter on a non-platform thread. Platform channel messages " + "must be sent on the platform thread. Failure to do so may " + "result in data loss or crashes, and must be fixed in the " + "plugin or application code creating that channel.\nSee " + "https://docs.flutter.dev/platform-integration/" + "platform-channels#channels-and-platform-threading for more " + "information.\n")); +} + } // namespace testing } // namespace flutter From a723d7f93950be398f458a4ce17cd8cf7f7f8d92 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 16 Jun 2023 13:53:44 -0700 Subject: [PATCH 2/7] review --- fml/logging.cc | 15 ++++++++++----- fml/logging.h | 5 ++++- fml/memory/thread_checker.cc | 2 +- fml/memory/thread_checker.h | 12 ++++++------ shell/common/shell.cc | 5 +++-- shell/common/shell.h | 3 ++- shell/common/shell_unittests.cc | 15 ++++++++++++++- 7 files changed, 40 insertions(+), 17 deletions(-) diff --git a/fml/logging.cc b/fml/logging.cc index b498483455c8d..0850ceaf9f2c9 100644 --- a/fml/logging.cc +++ b/fml/logging.cc @@ -73,12 +73,17 @@ LogMessage::LogMessage(LogSeverity severity, } // static -std::shared_ptr LogMessage::test_stream_ = nullptr; +thread_local std::shared_ptr + LogMessage::capture_next_log_stream_ = nullptr; + +void CaptureNextLog(const std::shared_ptr& stream) { + LogMessage::CaptureNextLog(stream); +} // static void LogMessage::CaptureNextLog( const std::shared_ptr& stream) { - test_stream_ = stream; + LogMessage::capture_next_log_stream_ = stream; } LogMessage::~LogMessage() { @@ -86,9 +91,9 @@ LogMessage::~LogMessage() { stream_ << std::endl; #endif - if (test_stream_) { - *test_stream_ << stream_.str(); - test_stream_.reset(); + if (capture_next_log_stream_) { + *capture_next_log_stream_ << stream_.str(); + capture_next_log_stream_.reset(); } else { #if defined(FML_OS_ANDROID) android_LogPriority priority = diff --git a/fml/logging.h b/fml/logging.h index 9596560ba857f..e87af970e4999 100644 --- a/fml/logging.h +++ b/fml/logging.h @@ -12,6 +12,8 @@ namespace fml { +void CaptureNextLog(const std::shared_ptr& stream); + class LogMessageVoidify { public: void operator&(std::ostream&) {} @@ -30,7 +32,8 @@ class LogMessage { static void CaptureNextLog(const std::shared_ptr& stream); private: - static std::shared_ptr test_stream_; + static thread_local std::shared_ptr + capture_next_log_stream_; std::ostringstream stream_; const LogSeverity severity_; const char* file_; diff --git a/fml/memory/thread_checker.cc b/fml/memory/thread_checker.cc index 77bbd3d6b9532..8a7e02385a107 100644 --- a/fml/memory/thread_checker.cc +++ b/fml/memory/thread_checker.cc @@ -6,6 +6,6 @@ namespace fml { -bool ThreadChecker::disable_next_ = false; +thread_local bool ThreadChecker::disable_next_failure_ = false; } diff --git a/fml/memory/thread_checker.h b/fml/memory/thread_checker.h index f0af15b06c638..ccf0f558139c5 100644 --- a/fml/memory/thread_checker.h +++ b/fml/memory/thread_checker.h @@ -36,8 +36,8 @@ class ThreadChecker final { bool IsCreationThreadCurrent() const { bool result = GetCurrentThreadId() == self_; - if (!result && disable_next_) { - disable_next_ = false; + if (!result && disable_next_failure_) { + disable_next_failure_ = false; return true; } return result; @@ -55,8 +55,8 @@ class ThreadChecker final { bool IsCreationThreadCurrent() const { pthread_t current_thread = pthread_self(); bool is_creation_thread_current = !!pthread_equal(current_thread, self_); - if (disable_next_ && !is_creation_thread_current) { - disable_next_ = false; + if (disable_next_failure_ && !is_creation_thread_current) { + disable_next_failure_ = false; return true; } #ifdef __APPLE__ @@ -78,10 +78,10 @@ class ThreadChecker final { return is_creation_thread_current; } - static void DisableNextThreadCheckFailure() { disable_next_ = true; } + static void DisableNextThreadCheckFailure() { disable_next_failure_ = true; } private: - static bool disable_next_; + static thread_local bool disable_next_failure_; pthread_t self_; #endif }; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 34dd9ef3ac0a4..859a2668c3762 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -992,13 +992,13 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { } } -std::set Shell::misbehaving_message_channels_ = {}; - // |PlatformView::Delegate| void Shell::OnPlatformViewDispatchPlatformMessage( std::unique_ptr message) { FML_DCHECK(is_setup_); +#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { + std::scoped_lock lock(misbehaving_message_channels_mutex_); auto inserted = misbehaving_message_channels_.insert(message->channel()); if (inserted.second) { FML_LOG(ERROR) @@ -1013,6 +1013,7 @@ void Shell::OnPlatformViewDispatchPlatformMessage( "information."; } } +#endif // FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG // The static leak checker gets confused by the use of fml::MakeCopyable. // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) diff --git a/shell/common/shell.h b/shell/common/shell.h index 8bee63d31b06c..d913c3e9c84a6 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -416,7 +416,8 @@ class Shell final : public PlatformView::Delegate, /// message from a non-platform thread. Used to prevent printing the error log /// more than once per channel, as a badly behaving plugin may send multiple /// messages per second indefinitely. - static std::set misbehaving_message_channels_; + std::mutex misbehaving_message_channels_mutex_; + std::set misbehaving_message_channels_; const TaskRunners task_runners_; const fml::RefPtr parent_raster_thread_merger_; std::shared_ptr diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index f22edee0dcd07..bc5ddb7b8dda9 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4233,6 +4233,9 @@ TEST_F(ShellTest, NotifyDestroyed) { } TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { +#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG + GTEST_SKIP() << "Test is for debug mode only."; +#endif Settings settings = CreateSettingsForFixture(); ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".", ThreadHost::Type::Platform); @@ -4242,7 +4245,7 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { auto shell = CreateShell(settings, task_runners); auto stream = std::make_shared(); - fml::LogMessage::CaptureNextLog(stream); + fml::CaptureNextLog(stream); // The next call will result in a thread checker violation. fml::ThreadChecker::DisableNextThreadCheckFailure(); @@ -4259,6 +4262,16 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { "https://docs.flutter.dev/platform-integration/" "platform-channels#channels-and-platform-threading for more " "information.\n")); + + stream = std::make_shared(); + fml::CaptureNextLog(stream); + + // The next call will result in a thread checker violation. + fml::ThreadChecker::DisableNextThreadCheckFailure(); + SendPlatformMessage(shell.get(), std::make_unique( + "com.test.plugin", nullptr)); + + EXPECT_EQ(stream->str(), ""); } } // namespace testing From 8306f4442b24147c6af3f1658353453c83c433e7 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 20 Jun 2023 15:32:27 -0700 Subject: [PATCH 3/7] Raw pointer --- fml/logging.cc | 10 ++++------ fml/logging.h | 9 +++++---- shell/common/shell_unittests.cc | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/fml/logging.cc b/fml/logging.cc index 0850ceaf9f2c9..bc5dd353f2120 100644 --- a/fml/logging.cc +++ b/fml/logging.cc @@ -73,16 +73,14 @@ LogMessage::LogMessage(LogSeverity severity, } // static -thread_local std::shared_ptr - LogMessage::capture_next_log_stream_ = nullptr; +thread_local std::ostringstream* LogMessage::capture_next_log_stream_ = nullptr; -void CaptureNextLog(const std::shared_ptr& stream) { +void CaptureNextLog(std::ostringstream* stream) { LogMessage::CaptureNextLog(stream); } // static -void LogMessage::CaptureNextLog( - const std::shared_ptr& stream) { +void LogMessage::CaptureNextLog(std::ostringstream* stream) { LogMessage::capture_next_log_stream_ = stream; } @@ -93,7 +91,7 @@ LogMessage::~LogMessage() { if (capture_next_log_stream_) { *capture_next_log_stream_ << stream_.str(); - capture_next_log_stream_.reset(); + capture_next_log_stream_ = nullptr; } else { #if defined(FML_OS_ANDROID) android_LogPriority priority = diff --git a/fml/logging.h b/fml/logging.h index e87af970e4999..21a5a5d82715d 100644 --- a/fml/logging.h +++ b/fml/logging.h @@ -12,7 +12,7 @@ namespace fml { -void CaptureNextLog(const std::shared_ptr& stream); +void CaptureNextLog(std::ostringstream* stream); class LogMessageVoidify { public: @@ -29,11 +29,12 @@ class LogMessage { std::ostream& stream() { return stream_; } - static void CaptureNextLog(const std::shared_ptr& stream); + static void CaptureNextLog(std::ostringstream* stream); private: - static thread_local std::shared_ptr - capture_next_log_stream_; + // This is a raw pointer so that we avoid having a non-trivially-destructible + // static. It is only ever for use in unit tests. + static thread_local std::ostringstream* capture_next_log_stream_; std::ostringstream stream_; const LogSeverity severity_; const char* file_; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index bc5ddb7b8dda9..e2e8e4af857be 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4245,7 +4245,7 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { auto shell = CreateShell(settings, task_runners); auto stream = std::make_shared(); - fml::CaptureNextLog(stream); + fml::CaptureNextLog(stream.get()); // The next call will result in a thread checker violation. fml::ThreadChecker::DisableNextThreadCheckFailure(); @@ -4264,7 +4264,7 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { "information.\n")); stream = std::make_shared(); - fml::CaptureNextLog(stream); + fml::CaptureNextLog(stream.get()); // The next call will result in a thread checker violation. fml::ThreadChecker::DisableNextThreadCheckFailure(); From a6b41cb9bbd2c58c8afd1b2a7cfdbca0193dc61c Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 20 Jun 2023 16:03:36 -0700 Subject: [PATCH 4/7] Fuchsia --- shell/common/shell_unittests.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index e2e8e4af857be..d3e5236876766 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4261,7 +4261,11 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { "plugin or application code creating that channel.\nSee " "https://docs.flutter.dev/platform-integration/" "platform-channels#channels-and-platform-threading for more " +#if !OS_FUCHSIA "information.\n")); +#else + "information.")); +#endif // !OS_FUCHSIA stream = std::make_shared(); fml::CaptureNextLog(stream.get()); From cdc9c36949683a4f7f0725ddfc0083abb305078a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 20 Jun 2023 16:04:33 -0700 Subject: [PATCH 5/7] Windows --- fml/memory/thread_checker.h | 1 + 1 file changed, 1 insertion(+) diff --git a/fml/memory/thread_checker.h b/fml/memory/thread_checker.h index ccf0f558139c5..cd70574ca47cb 100644 --- a/fml/memory/thread_checker.h +++ b/fml/memory/thread_checker.h @@ -44,6 +44,7 @@ class ThreadChecker final { } private: + static thread_local bool disable_next_failure_; DWORD self_; #else From 30c39ec5db4309c67bd133bf99c2950d15e830eb Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 20 Jun 2023 16:22:51 -0700 Subject: [PATCH 6/7] more windows --- fml/memory/thread_checker.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fml/memory/thread_checker.h b/fml/memory/thread_checker.h index cd70574ca47cb..6d5f2fe04046e 100644 --- a/fml/memory/thread_checker.h +++ b/fml/memory/thread_checker.h @@ -29,6 +29,12 @@ namespace fml { // |CalledOnValidThread()| that lies in Release builds seems bad. Moreover, // there's a small space cost to having even an empty class. ) class ThreadChecker final { + public: + static void DisableNextThreadCheckFailure() { disable_next_failure_ = true; } + + private: + static thread_local bool disable_next_failure_; + public: #if defined(FML_OS_WIN) ThreadChecker() : self_(GetCurrentThreadId()) {} @@ -44,7 +50,6 @@ class ThreadChecker final { } private: - static thread_local bool disable_next_failure_; DWORD self_; #else @@ -79,10 +84,7 @@ class ThreadChecker final { return is_creation_thread_current; } - static void DisableNextThreadCheckFailure() { disable_next_failure_ = true; } - private: - static thread_local bool disable_next_failure_; pthread_t self_; #endif }; From d2497b749d5912abe04b73f058223a6c1f36326f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 22 Jun 2023 10:16:53 -0700 Subject: [PATCH 7/7] Update shell_unittests.cc --- shell/common/shell_unittests.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index d3e5236876766..0e110aa8540ac 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4233,8 +4233,8 @@ TEST_F(ShellTest, NotifyDestroyed) { } TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { -#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG - GTEST_SKIP() << "Test is for debug mode only."; +#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DEBUG || OS_FUCHSIA + GTEST_SKIP() << "Test is for debug mode only on non-fuchsia targets."; #endif Settings settings = CreateSettingsForFixture(); ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".", @@ -4261,11 +4261,7 @@ TEST_F(ShellTest, PrintsErrorWhenPlatformMessageSentFromWrongThread) { "plugin or application code creating that channel.\nSee " "https://docs.flutter.dev/platform-integration/" "platform-channels#channels-and-platform-threading for more " -#if !OS_FUCHSIA "information.\n")); -#else - "information.")); -#endif // !OS_FUCHSIA stream = std::make_shared(); fml::CaptureNextLog(stream.get());