Enable aborting an Operation result, fix dead lock

src/ServerMain.cpp

@@ -144,9 +144,13 @@ int main(int argc, char** argv) {
     Server server(port, numThreads);
     server.initialize(index, text, allPermutations, usePatterns);
     server.run();
-  } catch (const ad_semsearch::Exception& e) {
-    LOG(ERROR) << e.getFullErrorMessage() << '\n';
+  } catch (const std::exception& e) {
+    // This code should never be reached as all exceptions should be handled
+    // within server.run()
+    LOG(ERROR) << e.what() << std::endl;
     return 1;
   }
-  return 0;
+  // This should also never be reached as the server threads are not supposed
+  // to terminate.
+  return 2;
 }

src/SparqlEngineMain.cpp

@@ -191,10 +191,7 @@ int main(int argc, char** argv) {
       }
     }
   } catch (const std::exception& e) {
-    cout << string("Caught exceptions: ") + e.what() << std::endl;
-    return 1;
-  } catch (ad_semsearch::Exception& e) {
-    cout << e.getFullErrorMessage() << std::endl;
+    cout << e.what() << std::endl;
   }
 
   return 0;

src/WriteIndexListsMain.cpp

@@ -112,10 +112,7 @@ int main(int argc, char** argv) {
     f.close();
 
   } catch (const std::exception& e) {
-    cout << string("Caught exceptions: ") + e.what();
-    return 1;
-  } catch (ad_semsearch::Exception& e) {
-    cout << e.getFullErrorMessage() << std::endl;
+    cout << e.what() << std::endl;
   }
 
   return 0;

src/engine/Operation.h

@@ -34,20 +34,51 @@ class Operation {
   // Use existing results if they are already available, otherwise
   // trigger computation.
   shared_ptr<const ResultTable> getResult() const {
-    LOG(DEBUG) << "Try to atomically emplace a new empty ResultTable" << endl;
+    LOG(DEBUG) << "Check cache for Operation result" << endl;
     LOG(DEBUG) << "Using key: \n" << asString() << endl;
     auto [newResult, existingResult] =
         _executionContext->getQueryTreeCache().tryEmplace(asString());
     if (newResult) {
-      LOG(DEBUG) << "We were the first to emplace, need to compute result"
-                 << endl;
+      LOG(DEBUG) << "Not in the cache, need to compute result" << endl;
       // Passing the raw pointer here is ok as the result shared_ptr remains
       // in scope
-      computeResult(newResult.get());
+      try {
+        computeResult(newResult.get());
+      } catch (const ad_semsearch::AbortException& e) {
+        newResult->abort();
+        // AbortExceptions have already been printed simply rethrow to
+        // unwind the callstack until the whole query is aborted
+        throw;
+      } catch (const std::exception& e) {
+        // Only print the Operation at the innermost (original) failure
+        // then "rethrow" as special ad_semsearch::AbortException
+        LOG(ERROR) << "Failed to compute Operation result for:" << endl;
+        LOG(ERROR) << asString() << endl;
+        LOG(ERROR) << e.what() << endl;
+        newResult->abort();
+        // Rethrow as QUERY_ABORTED allowing us to print the Operation
+        // only at innermost failure of a recursive call
+        throw ad_semsearch::AbortException(e);
+      } catch (...) {
+        // Only print the Operation at the innermost (original) failure
+        // then create not so weird AbortException
+        LOG(ERROR) << "Failed to compute Operation result for:" << endl;
+        LOG(ERROR) << asString() << endl;
+        LOG(ERROR) << "WEIRD_EXCEPTION not inheriting from std::exception"
+                   << endl;
+        newResult->abort();
+        // Rethrow as QUERY_ABORTED allowing us to print the Operation
+        // only at innermost failure of a recursive call
+        throw ad_semsearch::AbortException("WEIRD_EXCEPTION");
+      }
       return newResult;
     }
-    LOG(INFO) << "Result already (being) computed" << endl;
     existingResult->awaitFinished();
+    if (existingResult->status() == ResultTable::ABORTED) {
+      LOG(ERROR) << "Result in the cache was aborted" << endl;
+      AD_THROW(ad_semsearch::Exception::BAD_QUERY,
+               "Operation was found aborted in the cache");
+    }
     return existingResult;
   }
 

src/engine/ResultTable.cpp

@@ -13,7 +13,7 @@ ResultTable::ResultTable()
       _fixedSizeData(nullptr),
       _resultTypes(),
       _localVocab(std::make_shared<std::vector<std::string>>()),
-      _status(ResultTable::OTHER) {}
+      _status(ResultTable::IN_PROGRESS) {}
 
 // _____________________________________________________________________________
 void ResultTable::clear() {
@@ -45,8 +45,9 @@ void ResultTable::clear() {
     }
   }
   _fixedSizeData = nullptr;
+  _localVocab = nullptr;
   _varSizeData.clear();
-  _status = OTHER;
+  _status = IN_PROGRESS;
 }
 
 // _____________________________________________________________________________

src/engine/ResultTable.h

@@ -20,7 +20,7 @@ using std::vector;
 
 class ResultTable {
  public:
-  enum Status { FINISHED = 0, OTHER = 1 };
+  enum Status { IN_PROGRESS = 0, FINISHED = 1, ABORTED = 2 };
 
   /**
    * @brief Describes the type of a columns data
@@ -79,21 +79,27 @@ class ResultTable {
 
   virtual ~ResultTable();
 
+  void abort() {
+    lock_guard<mutex> lk(_cond_var_m);
+    clear();
+    _status = ResultTable::ABORTED;
+    _cond_var.notify_all();
+  }
+
   void finish() {
     lock_guard<mutex> lk(_cond_var_m);
     _status = ResultTable::FINISHED;
     _cond_var.notify_all();
   }
 
-  bool isFinished() const {
+  Status status() const {
     lock_guard<mutex> lk(_cond_var_m);
-    bool tmp = _status == ResultTable::FINISHED;
-    return tmp;
+    return _status;
   }
 
   void awaitFinished() const {
     unique_lock<mutex> lk(_cond_var_m);
-    _cond_var.wait(lk, [&] { return _status == ResultTable::FINISHED; });
+    _cond_var.wait(lk, [&] { return _status != ResultTable::IN_PROGRESS; });
   }
 
   std::optional<std::string> idToOptionalString(Id id) const {

src/engine/Server.h

@@ -38,7 +38,8 @@ class Server {
   void initialize(const string& ontologyBaseName, bool useText,
                   bool allPermutations = false, bool usePatterns = false);
 
-  //! Loop, wait for requests and trigger processing.
+  //! Loop, wait for requests and trigger processing. This method never returns
+  //! except when throwing an exceptiob
   void run();
 
  private:

src/util/Exception.h

@@ -6,6 +6,7 @@
 #ifndef GLOBALS_EXCEPTION_H_
 #define GLOBALS_EXCEPTION_H_
 
+#include <exception>
 #include <sstream>
 #include <string>
 
@@ -22,9 +23,6 @@ using std::string;
     throw ad_semsearch::Exception(e, __os.str(), __FILE__, __LINE__, \
                                   __PRETTY_FUNCTION__);              \
   }  // NOLINT
-// Rethrow an exception
-#define AD_RETHROW(e) \
-  throw semsearch::Exception(e.getErrorCode(), e.getErrorDetails())  // NOLINT
 
 // --------------------------------------------------------------------------
 // Macros for assertions that will throw Exceptions.
@@ -87,13 +85,29 @@ using std::string;
 // Exception class code
 // -------------------------------------------
 namespace ad_semsearch {
+
+//! Exception class for rethrowing exceptions during a query abort
+//! such exceptions are never printed but still keep the original what()
+//! message just in case
+class AbortException : public std::exception {
+ public:
+  AbortException(const std::exception& original) : _what(original.what()) {}
+
+  AbortException(const std::string& whatthe) : _what(whatthe) {}
+
+  const char* what() const noexcept { return _what.c_str(); }
+
+ private:
+  string _what;
+};
+
 //! Exception class for all kinds of exceptions.
 //! Compatibility with the THROW macro is ensured by using error
 //! codes inside this exception class instead of implementing an
 //! exception hierarchy through inheritance.
 //! This approach is taken from CompleteSearch's exception code.
 //! Add error codes whenever necessary.
-class Exception {
+class Exception : public std::exception {
  private:
   //! Error code
   int _errorCode;
@@ -102,7 +116,9 @@ class Exception {
   //! optionally provided by thrower)
   string _errorDetails;
 
-  string _errorDetailsNoFileAndLines;
+  string _errorDetailsFileAndLines;
+
+  string _errorMessageFull;
 
  public:
   //! Error codes
@@ -119,13 +135,13 @@ class Exception {
     // formatting errors
     BAD_INPUT = 16 * 2 + 5,
     BAD_REQUEST = 16 * 2 + 6,
-    BAD_QUERY = 16 * 2 + 7,
 
     // memory allocation errors
     REALLOC_FAILED = 16 * 3 + 1,
     NEW_FAILED = 16 * 3 + 2,
 
-    // intersect errors
+    // query errors
+    BAD_QUERY = 16 * 4 + 1,
 
     // history errors
 
@@ -147,7 +163,7 @@ class Exception {
   };
 
   //! Error messages (one per code)
-  const char* errorCodeAsString(int errorCode) const {
+  static const string errorCodeAsString(int errorCode) {
     switch (errorCode) {
       case VOCABULARY_MISS:
         return "VOCABULARY MISS";
@@ -166,8 +182,6 @@ class Exception {
         return "MEMORY ALLOCATION ERROR: new failed";
       case ERROR_PASSED_ON:
         return "PASSING ON ERROR";
-        return "QUERY EXCEPTION: "
-               "Check of query result failed";
       case UNCOMPRESS_ERROR:
         return "UNCOMPRESSION PROBLEM";
       case COULD_NOT_GET_MUTEX:
@@ -198,27 +212,28 @@ class Exception {
   explicit Exception(int errorCode) {
     _errorCode = errorCode;
     _errorDetails = "";
-    _errorDetailsNoFileAndLines = "";
+    _errorDetailsFileAndLines = "";
+    _errorMessageFull = errorCodeAsString(_errorCode);
   }
 
   //! Constructor (code + details)
-  Exception(int errorCode, string errorDetails) {
+  Exception(int errorCode, const string& errorDetails) {
     _errorCode = errorCode;
     _errorDetails = errorDetails;
-    _errorDetailsNoFileAndLines = errorDetails;
+    _errorDetailsFileAndLines = "";
+    _errorMessageFull = getErrorMessage() + " (" + _errorDetails + ")";
   }
 
   //! Constructor
   //! (code + details + file name + line number + enclosing method)
-  Exception(int errorCode, const string& errorDetails, const char* file_name,
-            int line_no, const char* fct_name) {
+  Exception(int errorCode, const string& errorDetails, const string& file_name,
+            int line_no, const string& fct_name) {
     _errorCode = errorCode;
-    _errorDetailsNoFileAndLines = errorDetails;
-    std::ostringstream os;
-    if (errorDetails.size() > 0) os << errorDetails << "; ";
-    os << "in " << file_name << ", line " << line_no << ", function "
-       << fct_name;
-    _errorDetails = os.str();
+    _errorDetails = errorDetails;
+    _errorDetailsFileAndLines = "in " + file_name + ", line " +
+                                std::to_string(line_no) + ", function " +
+                                fct_name;
+    _errorMessageFull = getErrorMessage() + " (" + getErrorDetails() + ")";
   }
 
   //! Set error code
@@ -227,28 +242,30 @@ class Exception {
   //! Set error details
   void setErrorDetails(const string& errorDetails) {
     _errorDetails = errorDetails;
-    _errorDetailsNoFileAndLines = _errorDetailsNoFileAndLines;
   }
 
   //! Get error Code
-  int getErrorCode() const { return _errorCode; }
+  int getErrorCode() const noexcept { return _errorCode; }
 
   //! Get error message pertaining to code
-  string getErrorMessage() const { return errorCodeAsString(_errorCode); }
+  string getErrorMessage() const noexcept {
+    return errorCodeAsString(_errorCode);
+  }
 
   //! Get error details
-  const string& getErrorDetails() const { return _errorDetails; }
+  const string getErrorDetails() const noexcept {
+    return _errorDetails + "; " + _errorDetailsFileAndLines;
+  }
 
-  //! Get full error message (generic message + specific details if available)
-  string getFullErrorMessage() const {
-    return _errorDetails.length() > 0
-               ? getErrorMessage() + " (" + _errorDetails + ")"
-               : getErrorMessage();
+  const string getErrorDetailsNoFileAndLines() const noexcept {
+    return _errorDetails;
   }
 
-  const string& getErrorMsgNoFileAndLines() const {
-    return _errorDetailsNoFileAndLines;
+  const string& getFullErrorMessage() const noexcept {
+    return _errorMessageFull;
   }
+
+  const char* what() const noexcept { return _errorMessageFull.c_str(); }
 };
 }  // namespace ad_semsearch
 

test/QueryPlannerTest.cpp

@@ -673,7 +673,7 @@ TEST(QueryPlannerTest, threeVarXthreeVarException) {
         "Could not find a suitable execution tree. "
         "Likely cause: Queries that require joins of the full index with "
         "itself are not supported at the moment.",
-        e.getErrorMsgNoFileAndLines());
+        e.getErrorDetailsNoFileAndLines());
   } catch (const std::exception& e) {
     std::cout << "Caught: " << e.what() << std::endl;
     FAIL() << e.what();