Skip to content

Commit

Permalink
Make FML_LOG safe from static initialization (flutter#42219)
Browse files Browse the repository at this point in the history
I ran into this while trying to get some printing going for places where we're creating thread local keys. 

Supposedly, just including `<iostream>` should statically initialize `std::cout/cerr`, but it gets really hard to reason about whether your statically initialized code is going to be initialized before or after that happens. I tried making sure that the TU for `fml/logging.cc` did that initialization statically, but that also failed in the verison of the test included here (it passed in some other iterations that modified run_all_unittests.cc). We _could_ make sure it happens each and every time we touch `std::cerr` but ... we could also just use `fprintf(stderr, ...)` and it works just fine.

/cc @flar who ran into problems around this a little while back and was asking about it.
  • Loading branch information
dnfield authored May 23, 2023
1 parent a1101e9 commit 41e8d52
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
5 changes: 3 additions & 2 deletions fml/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ LogMessage::~LogMessage() {
fx_logger_log_with_source(fx_log_get_logger(), fx_severity, nullptr, file_,
line_, stream_.str().c_str());
#else
std::cerr << stream_.str();
std::cerr.flush();
// 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) {
Expand Down
37 changes: 37 additions & 0 deletions fml/logging_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <signal.h>

#include "flutter/fml/build_config.h"
#include "flutter/fml/log_settings.h"
#include "flutter/fml/logging.h"
Expand All @@ -19,6 +21,41 @@
namespace fml {
namespace testing {

#ifndef OS_FUCHSIA
class MakeSureFmlLogDoesNotSegfaultWhenStaticallyCalled {
public:
MakeSureFmlLogDoesNotSegfaultWhenStaticallyCalled() {
SegfaultCatcher catcher;
// If this line causes a segfault, FML is using a method of logging that is
// not safe from static initialization on your platform.
FML_LOG(INFO)
<< "This log exists to verify that static logging from FML works.";
}

private:
struct SegfaultCatcher {
typedef void (*sighandler_t)(int);

SegfaultCatcher() {
handler = ::signal(SIGSEGV, SegfaultHandler);
FML_CHECK(handler != SIG_ERR);
}

~SegfaultCatcher() { FML_CHECK(::signal(SIGSEGV, handler) != SIG_ERR); }

static void SegfaultHandler(int signal) {
fprintf(stderr,
"FML failed to handle logging from static initialization.\n");
exit(signal);
}

sighandler_t handler;
};
};

static MakeSureFmlLogDoesNotSegfaultWhenStaticallyCalled fml_log_static_check_;
#endif // !defined(OS_FUCHSIA)

int UnreachableScopeWithoutReturnDoesNotMakeCompilerMad() {
KillProcess();
// return 0; <--- Missing but compiler is fine.
Expand Down

0 comments on commit 41e8d52

Please sign in to comment.