Refactor: Make it clear that we only support UNIX

So that we won’t have any indirections in code regarding signals etc.
ia-toki · May 24, 2017 · 1abf62a · 1abf62a
1 parent d7e823f
commit 1abf62a
Show file tree

Hide file tree

Showing 14 changed files with 120 additions and 164 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -86,7 +86,6 @@ set(INCLUDE
         include/tcframe/os/ExecutionResult.hpp
         include/tcframe/os/ExecutionRequest.hpp
         include/tcframe/os/OperatingSystem.hpp
-        include/tcframe/os/UnixOperatingSystem.hpp
         include/tcframe/runner.hpp
         include/tcframe/runner/Args.hpp
         include/tcframe/runner/ArgsParser.hpp
@@ -232,7 +231,7 @@ target_link_libraries(test_unit
 set(TEST_INTEGRATION
         test/integration/tcframe/evaluator/scorer/CustomScorerIntegrationTests.cpp
         test/integration/tcframe/evaluator/scorer/DiffScorerIntegrationTests.cpp
-        test/integration/tcframe/os/UnixOperatingSystemIntegrationTests.cpp
+        test/integration/tcframe/os/OperatingSystemIntegrationTests.cpp
 )
 
 add_executable(test_integration ${INCLUDE} ${TEST_INTEGRATION})

diff --git a/include/tcframe/logger/BaseLogger.hpp b/include/tcframe/logger/BaseLogger.hpp
@@ -1,13 +1,16 @@
 #pragma once
 
 #include <map>
+#include <string>
+#include <sys/signal.h>
 
 #include "LoggerEngine.hpp"
 #include "tcframe/os.hpp"
 #include "tcframe/spec/testcase.hpp"
 #include "tcframe/util.hpp"
 
 using std::map;
+using std::string;
 
 namespace tcframe {
 
@@ -46,7 +49,7 @@ class BaseLogger {
                     engine_->logListItem2(3, "Exit code: " + StringUtils::toString(executionResult.exitCode().value()));
                     engine_->logListItem2(3, "Standard error: " + executionResult.standardError());
                 } else {
-                    engine_->logListItem2(3, "Exit signal: " + executionResult.exitSignal().value());
+                    engine_->logListItem2(3, "Exit signal: " + string(strsignal(executionResult.exitSignal().value())));
                 }
             } else if (!executionResult.standardError().empty()) {
                 engine_->logListItem1(2, key + ": " + executionResult.standardError());

diff --git a/include/tcframe/os.hpp b/include/tcframe/os.hpp
@@ -3,4 +3,3 @@
 #include "tcframe/os/ExecutionResult.hpp"
 #include "tcframe/os/ExecutionRequest.hpp"
 #include "tcframe/os/OperatingSystem.hpp"
-#include "tcframe/os/UnixOperatingSystem.hpp"
diff --git a/include/tcframe/os/ExecutionResult.hpp b/include/tcframe/os/ExecutionResult.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+#include <sys/signal.h>
 #include <tuple>
 #include <utility>
 
@@ -17,27 +18,21 @@ struct ExecutionResult {
 
 private:
     optional<int> exitCode_;
-    optional<string> exitSignal_;
-    bool exceededCpuLimits_;
+    optional<int> exitSignal_;
     string standardError_;
 
 public:
     ExecutionResult()
-            : exitCode_(optional<int>(0))
-            , exceededCpuLimits_(false) {}
+            : exitCode_(optional<int>(0)) {}
 
     const optional<int>& exitCode() const {
         return exitCode_;
     }
 
-    const optional<string>& exitSignal() const {
+    const optional<int>& exitSignal() const {
         return exitSignal_;
     }
 
-    bool exceededCpuLimits() const {
-        return exceededCpuLimits_;
-    }
-
     const string& standardError() const {
         return standardError_;
     }
@@ -47,8 +42,8 @@ struct ExecutionResult {
     }
 
     bool operator==(const ExecutionResult& o) const {
-        return tie(exitCode_, exitSignal_, exceededCpuLimits_, standardError_)
-               == tie(o.exitCode_, o.exitSignal_, o.exceededCpuLimits_, standardError_);
+        return tie(exitCode_, exitSignal_, standardError_)
+               == tie(o.exitCode_, o.exitSignal_, standardError_);
     }
 };
 
@@ -64,18 +59,13 @@ class ExecutionResultBuilder {
 
     ExecutionResultBuilder& setExitCode(int exitCode) {
         subject_.exitCode_ = optional<int>(exitCode);
-        subject_.exitSignal_ = optional<string>();
+        subject_.exitSignal_ = optional<int>();
         return *this;
     }
 
-    ExecutionResultBuilder& setExitSignal(string exitSignal) {
+    ExecutionResultBuilder& setExitSignal(int exitSignal) {
         subject_.exitCode_ = optional<int>();
-        subject_.exitSignal_ = optional<string>(exitSignal);
-        return *this;
-    }
-
-    ExecutionResultBuilder& setExceededCpuLimits(bool exceededCpuLimits) {
-        subject_.exceededCpuLimits_ = exceededCpuLimits;
+        subject_.exitSignal_ = optional<int>(exitSignal);
         return *this;
     }
 

diff --git a/include/tcframe/os/OperatingSystem.hpp b/include/tcframe/os/OperatingSystem.hpp
@@ -1,29 +1,105 @@
 #pragma once
 
-#include <istream>
-#include <ostream>
+#include <cstring>
+#include <fstream>
+#include <iostream>
+#include <sstream>
 #include <string>
+#include <sys/wait.h>
 
 #include "ExecutionRequest.hpp"
 #include "ExecutionResult.hpp"
+#include "tcframe/util.hpp"
 
+using std::ifstream;
 using std::ios;
 using std::istream;
+using std::ofstream;
 using std::ostream;
+using std::ostringstream;
 using std::string;
 
 namespace tcframe {
 
 class OperatingSystem {
+private:
+    static constexpr const char* ERROR_FILENAME = "_error.out";
+
 public:
-    virtual ~OperatingSystem() {}
-
-    virtual istream* openForReading(const string& filename) = 0;
-    virtual ostream* openForWriting(const string& filename) = 0;
-    virtual void closeOpenedStream(ios* stream) = 0;
-    virtual void forceMakeDir(const string& dirName) = 0;
-    virtual void removeFile(const string& filename) = 0;
-    virtual ExecutionResult execute(const ExecutionRequest& request) = 0;
+    virtual istream* openForReading(const string& filename) {
+        ifstream* file = new ifstream();
+        file->open(filename);
+        return file;
+    }
+
+    virtual ostream* openForWriting(const string& filename) {
+        ofstream* file = new ofstream();
+        file->open(filename);
+        return file;
+    }
+
+    virtual void closeOpenedStream(ios* stream) {
+        delete stream;
+    }
+
+    virtual void forceMakeDir(const string& dirName) {
+        runCommand("rm -rf " + dirName);
+        runCommand("mkdir -p " + dirName);
+    }
+
+    virtual void removeFile(const string& filename) {
+        runCommand("rm -rf " + filename);
+    }
+
+    virtual ExecutionResult execute(const ExecutionRequest& request) {
+        ostringstream sout;
+
+        sout << "{ ";
+
+        if (request.timeLimit()) {
+            sout << "ulimit -S -t " << request.timeLimit().value() << "; ";
+        }
+
+        // Note: this has no effect on OS X. No known workaround unfortunately.
+        if (request.memoryLimit()) {
+            sout << "ulimit -S -v " << request.memoryLimit().value() * 1024 << "; ";
+        }
+
+        sout << request.command() << "; }";
+
+        if (request.inputFilename()) {
+            sout << " < " << request.inputFilename().value();
+        }
+        if (request.outputFilename()) {
+            sout << " 1> " << request.outputFilename().value();
+        } else {
+            sout << " 1> /dev/null";
+        }
+        sout << " 2> " << ERROR_FILENAME;
+
+        ExecutionResultBuilder result;
+
+        int exitValue = system(sout.str().c_str());
+        int exitStatus = WEXITSTATUS(exitValue);
+
+        if (exitStatus & (1<<7)) {
+            result.setExitSignal(WTERMSIG(exitStatus));
+        } else {
+            result.setExitCode(exitStatus);
+        }
+
+        istream* errorStream = openForReading(ERROR_FILENAME);
+        string errorString = StringUtils::streamToString(errorStream);
+        closeOpenedStream(errorStream);
+        result.setStandardError(errorString);
+
+        return result.build();
+    }
+
+private:
+    static void runCommand(const string& command) {
+        system(command.c_str());
+    }
 };
 
 }
diff --git a/include/tcframe/os/UnixOperatingSystem.hpp b/include/tcframe/os/UnixOperatingSystem.hpp
diff --git a/include/tcframe/verdict/VerdictCreator.hpp b/include/tcframe/verdict/VerdictCreator.hpp
@@ -3,6 +3,7 @@
 #include <istream>
 #include <stdexcept>
 #include <string>
+#include <sys/signal.h>
 
 #include "Verdict.hpp"
 #include "tcframe/os.hpp"
@@ -48,7 +49,7 @@ class VerdictCreator {
     }
 
     virtual optional<Verdict> fromExecutionResult(const ExecutionResult& executionResult) {
-        if (executionResult.exceededCpuLimits()) {
+        if (executionResult.exitSignal() == optional<int>(SIGXCPU)) {
             return optional<Verdict>(Verdict(VerdictStatus::tle()));
         } else if (!executionResult.isSuccessful()) {
             return optional<Verdict>(Verdict(VerdictStatus::rte()));

diff --git a/src/tcframe/runner.cpp b/src/tcframe/runner.cpp
@@ -15,7 +15,7 @@ int main(int argc, char* argv[]) {
             __TCFRAME_SPEC_FILE__,
             new TestSpec(),
             new SimpleLoggerEngine(),
-            new UnixOperatingSystem(),
+            new OperatingSystem(),
             new RunnerLoggerFactory(),
             new GeneratorFactory(),
             new GraderFactory(),

diff --git a/test/integration/tcframe/evaluator/scorer/CustomScorerIntegrationTests.cpp b/test/integration/tcframe/evaluator/scorer/CustomScorerIntegrationTests.cpp
@@ -15,7 +15,7 @@ namespace tcframe {
 class CustomScorerIntegrationTests : public Test {
 protected:
     CustomScorer scorer = CustomScorer(
-            new UnixOperatingSystem(),
+            new OperatingSystem(),
             new VerdictCreator(),
             "test-integration/evaluator/scorer/custom/scorer");
 

diff --git a/test/integration/tcframe/evaluator/scorer/DiffScorerIntegrationTests.cpp b/test/integration/tcframe/evaluator/scorer/DiffScorerIntegrationTests.cpp
@@ -13,7 +13,7 @@ namespace tcframe {
 
 class DiffScorerIntegrationTests : public Test {
 protected:
-    DiffScorer scorer = DiffScorer(new UnixOperatingSystem());
+    DiffScorer scorer = DiffScorer(new OperatingSystem());
 };
 
 TEST_F(DiffScorerIntegrationTests, Scoring_AC) {