Skip to content

Commit d59dff6

Browse files
nickdowellnickdowell
committed
Fix CrashHandler & make async-signal-safe
1 parent ed19c2c commit d59dff6

3 files changed

Lines changed: 112 additions & 60 deletions

File tree

‎Source/CommandLine.cpp‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include "CrashHandler.h"
1818

1919
#if JUCE_MAC
20-
#include <signal.h>
2120
#include <sys/types.h>
2221
#include <unistd.h>
2322
#endif
@@ -56,37 +55,10 @@ inline void logAndFlush (const juce::String& m)
5655
std::cout << m << std::flush;
5756
}
5857

59-
//==============================================================================
60-
#if JUCE_MAC
61-
static void kill9WithSomeMercy (int signal)
62-
{
63-
juce::Logger::writeToLog ("pluginval received " + juce::String(::strsignal(signal)) + ", exiting immediately");
64-
65-
// Use std::_Exit here instead of kill as kill doesn't seem to set the exit code of the process so is picked up as a "pass" in the host process
66-
std::_Exit (SIGKILL);
67-
}
68-
69-
// Avoid showing the macOS crash dialog, which can cause the process to hang
70-
static void setupSignalHandling()
71-
{
72-
const int signals[] = { SIGFPE, SIGILL, SIGSEGV, SIGBUS, SIGABRT };
73-
74-
for (int i = 0; i < juce::numElementsInArray (signals); ++i)
75-
{
76-
::signal (signals[i], kill9WithSomeMercy);
77-
::siginterrupt (signals[i], 1);
78-
}
79-
}
80-
#endif
81-
82-
8358
//==============================================================================
8459
//==============================================================================
8560
CommandLineValidator::CommandLineValidator()
8661
{
87-
#if JUCE_MAC
88-
setupSignalHandling();
89-
#endif
9062
}
9163

9264
CommandLineValidator::~CommandLineValidator()

‎Source/CrashHandler.cpp‎

Lines changed: 109 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,57 +15,134 @@
1515
#include "juce_core/juce_core.h"
1616
#include "CrashHandler.h"
1717

18-
#if JUCE_MAC
18+
#if !JUCE_WINDOWS
1919
#include <dlfcn.h>
20+
#include <execinfo.h>
21+
#include <fcntl.h>
22+
#include <stdarg.h>
23+
#include <stdio.h>
24+
#include <unistd.h>
25+
#define CRASH_LOG "/tmp/pluginval_crash.txt"
2026
#endif
2127

2228
namespace
2329
{
24-
juce::String getExtraPlatformSpecificInfo()
25-
{
26-
#if JUCE_MAC
27-
juce::StringArray imagesDone;
28-
juce::StringArray output;
29-
30-
for (auto& l : juce::StringArray::fromLines (juce::SystemStats::getStackBacktrace()))
31-
{
32-
const juce::String imageName = l.upToFirstOccurrenceOf ("0x", false, false).fromFirstOccurrenceOf (" ", false, false).trim();
33-
const juce::String addressString = l.fromFirstOccurrenceOf ("0x", true, false).upToFirstOccurrenceOf (" ", false, false).trim();
34-
35-
if (imagesDone.contains (imageName))
36-
continue;
37-
38-
imagesDone.add (imageName);
39-
40-
Dl_info info;
41-
const size_t address = static_cast<size_t> (addressString.getHexValue64());
42-
43-
if (dladdr (reinterpret_cast<void*> (address), &info) != 0)
44-
output.add (juce::String ("0x") + juce::String::toHexString (static_cast<juce::pointer_sized_int> (reinterpret_cast<size_t> (info.dli_fbase))) + " " + imageName);
45-
}
46-
47-
return "Binary Images:\n" + output.joinIntoString ("\n");
48-
#else
49-
return {};
50-
#endif
51-
}
52-
5330
juce::String getCrashLogContents()
5431
{
55-
return "\n" + juce::SystemStats::getStackBacktrace() + "\n" + getExtraPlatformSpecificInfo();
32+
return "\n" + juce::SystemStats::getStackBacktrace();
5633
}
5734

5835
static juce::File getCrashTraceFile()
5936
{
37+
#if JUCE_WINDOWS
6038
return juce::File::getSpecialLocation (juce::File::tempDirectory).getChildFile ("pluginval_crash.txt");
39+
#else
40+
return juce::File(CRASH_LOG);
41+
#endif
6142
}
6243

63-
static void handleCrash (void*)
44+
#if JUCE_WINDOWS
45+
static void handleCrash(void*)
6446
{
6547
const auto log = getCrashLogContents();
6648
std::cout << "\n*** FAILED: VALIDATION CRASHED\n" << log << std::endl;
6749
getCrashTraceFile().replaceWithText (log);
6850
}
51+
#else
52+
#ifdef __printflike
53+
__printflike(2, 3)
54+
#endif
55+
static void writeToCrashLog(int fd, const char* fmt, ...)
56+
{
57+
char buf[1024];
58+
va_list args;
59+
va_start(args, fmt);
60+
// Warning: printf is not 100% async-signal-safe, but should be ok for locale-independent arguments like
61+
// integers, strings, hex... floating point is locale-dependent and not safe to use here.
62+
vsnprintf(buf, sizeof(buf), fmt, args);
63+
va_end(args);
64+
auto len = strlen(buf);
65+
write(STDERR_FILENO, buf, len);
66+
if (fd != -1)
67+
write(fd, buf, len);
68+
}
69+
70+
static void handleCrash(void*)
71+
{
72+
// On Linux & Mac this is a signal handler, and therefore only "async-signal-safe" functions should be used.
73+
// This means nothing that uses malloc (juce::File, juce::String, std::string, std::vector etc.) or buffered I/O.
74+
75+
int fd = open(CRASH_LOG, O_RDWR | O_CREAT | O_TRUNC);
76+
77+
const char *message = "\n*** FAILED: VALIDATION CRASHED\n";
78+
write(STDERR_FILENO, message, strlen(message));
79+
80+
// Recreate the output of backtrace_symbols(), which cannot be used in a signal handler because it uses malloc
81+
static const int kMaxStacks = 128;
82+
void *stacktrace[kMaxStacks] {};
83+
int stackCount = backtrace(stacktrace, kMaxStacks);
84+
85+
static const int kMaxImages = 64;
86+
const void *imageAddresses[kMaxImages] {};
87+
const char *imageNames[kMaxImages] {};
88+
int imageCount = 0;
89+
90+
int skip = 2; // Skip handleCrash and juce::handleCrash)
91+
for (int i = skip; i < stackCount; i++)
92+
{
93+
Dl_info info {};
94+
// Warning: dladdr can deadlock under rare conditions on macOS - if dyld is adding an image to its list
95+
if (!dladdr(stacktrace[i], &info))
96+
{
97+
writeToCrashLog(fd, "%-3d %-35s %p\n", i - skip, "", stacktrace[i]);
98+
continue;
99+
}
100+
101+
const char *imageName = info.dli_fname ? strrchr(info.dli_fname, '/') : nullptr;
102+
if (imageName)
103+
{
104+
imageName++;
105+
106+
auto it = std::find(std::begin(imageAddresses), std::end(imageAddresses), info.dli_fbase);
107+
if (it == std::end(imageAddresses) && imageCount < kMaxImages)
108+
{
109+
imageAddresses[imageCount] = info.dli_fbase;
110+
imageNames[imageCount] = imageName;
111+
imageCount++;
112+
}
113+
}
114+
115+
if (info.dli_saddr)
116+
{
117+
ptrdiff_t offset = (char *)stacktrace[i] - (char *)info.dli_saddr;
118+
writeToCrashLog(fd, "%-3d %-35s %p %s + %ld\n", i - skip, imageName, stacktrace[i], info.dli_sname, offset);
119+
}
120+
else
121+
{
122+
writeToCrashLog(fd, "%-3d %-35s %p\n", i - skip, imageName, stacktrace[i]);
123+
}
124+
}
125+
126+
if (imageCount)
127+
{
128+
writeToCrashLog(fd, "\nBinary Images:");
129+
for (int i = 0; i < imageCount; i++)
130+
writeToCrashLog(fd, "\n%p %s", imageAddresses[i], imageNames[i]);
131+
writeToCrashLog(fd, "\n");
132+
}
133+
134+
if (fd != -1)
135+
close(fd);
136+
137+
// Terminate normally to work around a bug in juce::ChildProcess::ActiveProcess::getExitStatus()
138+
// which returns 0 (a "pass" in the host process) if the child process terminates abnormally.
139+
// - https://github.com/Tracktion/pluginval/issues/125
140+
// - https://forum.juce.com/t/killed-childprocess-activeprocess-exit-code/61645/3
141+
// Use _Exit() instead of exit() so that static destructors don't run (they may not be async-signal-safe).
142+
// FIXME: exiting here prevents Apple's Crash Reporter from creating reports.
143+
std::_Exit(SIGKILL);
144+
}
145+
#endif
69146
}
70147

71148
//==============================================================================

‎Source/Validator.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ struct PluginsUnitTestRunner : public juce::UnitTestRunner,
3232
jassert (callback);
3333
resetTimeout();
3434

35+
// Initialise the crash handler to clear any previous crash logs
36+
initialiseCrashHandler();
37+
3538
if (timeoutInMs > 0)
3639
startThread (juce::Thread::Priority::low);
3740
}

0 commit comments

Comments
 (0)