Use report_fatal_error before destroying input object on error.

src/IceBrowserCompileServer.cpp

Index: src/IceBrowserCompileServer.cpp
diff --git a/src/IceBrowserCompileServer.cpp b/src/IceBrowserCompileServer.cpp
index 92d42e28e2aaa4efc13e9c37236669f10efd04d5..ba11e0125491c850abeaa4082f0b8726e75233ca 100644
--- a/src/IceBrowserCompileServer.cpp
+++ b/src/IceBrowserCompileServer.cpp
@@ -18,6 +18,7 @@
 #include <cstring>
 #include <irt.h>
 #include <irt_dev.h>
+#include <pthread.h>
 #include <thread>
 
 #include "llvm/Support/QueueStreamer.h"
@@ -78,8 +79,7 @@ int onDataCallback(const void *Data, size_t NumBytes) {
 char *onEndCallback() {
   gCompileServer->endInputStream();
   gCompileServer->waitForCompileThread();
-  // TODO(jvoung): Also return an error string, and UMA data.
-  // Set up a report_fatal_error handler to grab that string.
+  // TODO(jvoung): Also return UMA data.
   if (gCompileServer->getErrorCode().value()) {
     const std::string Error = gCompileServer->getErrorStream().getContents();
     return strdup(Error.empty() ? "Some error occurred" : Error.c_str());
@@ -100,15 +100,28 @@ std::unique_ptr<llvm::raw_fd_ostream> getOutputStream(int FD) {
       new llvm::raw_fd_ostream(FD, CloseOnDtor, Unbuffered));
 }
 
+void fatalErrorHandler(void *UserData, const std::string &Reason,
+                       bool GenCrashDialog) {
+  BrowserCompileServer *Server =
+      reinterpret_cast<BrowserCompileServer *>(UserData);
+  Server->setFatalError(Reason);
+  // Only kill the current thread instead of the whole process.
+  // We need the server thread to remain alive in order to respond with the
+  // error message.
+  // We could also try to pthread_kill all other worker threads, but
+  // pthread_kill / raising signals is not supported by NaCl.
+  // We'll have to assume that the worker/emitter threads will be well behaved
+  // after a fatal error in other threads, and either get stuck waiting
+  // on input from a previous stage, or also call report_fatal_error.
+  pthread_exit(0);
+}
+
 } // end of anonymous namespace
 
 BrowserCompileServer::~BrowserCompileServer() {}
 
 void BrowserCompileServer::run() {
   gCompileServer = this;
-  // TODO(jvoung): make a useful fatal error handler that can
-  // exit all *worker* threads, keep the server thread alive,
-  // and be able to handle a server request for the last error string.
   getIRTInterfaces();
   gIRTFuncs.serve_translate_request(&SubzeroCallbacks);
 }
@@ -129,12 +142,33 @@ void BrowserCompileServer::getParsedFlags(uint32_t NumThreads, int argc,
 }
 
 bool BrowserCompileServer::pushInputBytes(const void *Data, size_t NumBytes) {
+  // If there was an earlier error, do not attempt to push bytes to
+  // the QueueStreamer. Otherwise the thread could become blocked.
+  if (HadError.load())
+    return true;
   return InputStream->PutBytes(
              const_cast<unsigned char *>(
                  reinterpret_cast<const unsigned char *>(Data)),
              NumBytes) != NumBytes;
 }
 
+void BrowserCompileServer::setFatalError(const IceString &Reason) {
+  HadError.store(true);
+  Ctx->getStrError() << Reason;

[Jim Stichnoth, 2015/06/03 16:16:01]

I was wondering about using OstreamLocker for multithreading safety/consistency,
but I guess that would be bad if an error were thrown while the lock was already
being held.

[jvoung (off chromium), 2015/06/04 00:33:16]

Hmm good point. I forgot about OstreamLocker, but yeah it would be bad to
deadlock here.

E.g., if the code in
https://codereview.chromium.org/1168543002/diff/1/src/PNaClTranslator.cpp, had
acquired OstreamLocker and then called report_fatal_error (also forgot to get
OstreamLocker there).

+  // Make sure that the QueueStreamer is not stuck by signaling an early end.
+  InputStream->SetDone();
+}
+
+ErrorCode &BrowserCompileServer::getErrorCode() {
+  if (HadError.load()) {
+    // HadError means report_fatal_error is called. Make sure that the
+    // LastError is not EC_None. We don't know the type of error so
+    // just pick some error category.
+    LastError.assign(EC_Translation);
+  }
+  return LastError;
+}
+
 void BrowserCompileServer::endInputStream() { InputStream->SetDone(); }
 
 void BrowserCompileServer::startCompileThread(int ObjFD) {
@@ -150,6 +184,7 @@ void BrowserCompileServer::startCompileThread(int ObjFD) {
                               &ErrorStream->getStream(), ELFStream.get(),
                               Flags));
   CompileThread = std::thread([this]() {
+    llvm::install_fatal_error_handler(fatalErrorHandler, this);
     Ctx->initParserThread();
     this->getCompiler().run(ExtraFlags, *Ctx.get(),
                             // Retain original reference, but the compiler