feat(grader): give WA when communicator received signal 13 before wri…

…ting verdict

judgels-backends/judgels-grader-engines/src/integTest/java/judgels/gabriel/engines/BlackboxGradingEngineIntegrationTests.java

@@ -149,7 +149,6 @@ protected TestCaseResult testCaseResult(
                 .time(100)
                 .memory(1000)
                 .status(s)
-                .message("OK")
                 .build());
         return new TestCaseResult.Builder()
                 .verdict(verdict)

judgels-backends/judgels-grader-engines/src/integTest/java/judgels/gabriel/engines/interactive/InteractiveGradingEngineIntegrationTests.java

@@ -9,6 +9,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import java.util.List;
 import judgels.gabriel.api.EvaluationException;
 import judgels.gabriel.api.GradingException;
 import judgels.gabriel.api.GradingResult;
@@ -155,4 +156,80 @@ void err_because_communicator_not_specified() {
                 .isInstanceOf(PreparationException.class)
                 .hasMessageContaining("Communicator not specified");
     }
+
+    @Test
+    void when_communicator_received_sigpipe_before_verdict_then_we_should_return_wa() throws GradingException {
+        addSourceFile("source", "trigger-communicator-sigpipe.cpp");
+        assertResult(
+                new InteractiveGradingConfig.Builder().from(CONFIG)
+                        .communicator("communicator-sigpipe-before-verdict.cpp").build(),
+                WRONG_ANSWER,
+                0,
+                List.of(
+                        testGroupResult(
+                                0,
+                                testCaseResult(WRONG_ANSWER, "", 0),
+                                testCaseResult(WRONG_ANSWER, "", 0),
+                                testCaseResult(WRONG_ANSWER, "", 0)),
+                        testGroupResult(
+                                -1,
+                                testCaseResult(WRONG_ANSWER, "0.0", -1),
+                                testCaseResult(WRONG_ANSWER, "0.0", -1),
+                                testCaseResult(WRONG_ANSWER, "0.0", -1),
+                                testCaseResult(WRONG_ANSWER, "0.0", -1),
+                                testCaseResult(WRONG_ANSWER, "0.0", -1))),
+                List.of(
+                        subtaskResult(-1, WRONG_ANSWER, 0)));
+    }
+
+
+    @Test
+    void when_communicator_received_sigpipe_after_verdict_then_we_should_still_return_the_verdict() throws GradingException {
+        addSourceFile("source", "trigger-communicator-sigpipe.cpp");
+        assertResult(
+                new InteractiveGradingConfig.Builder().from(CONFIG)
+                        .communicator("communicator-sigpipe-after-verdict.cpp").build(),
+                ACCEPTED,
+                100,
+                List.of(
+                        testGroupResult(
+                                0,
+                                testCaseResult(ACCEPTED, "", 0),
+                                testCaseResult(ACCEPTED, "", 0),
+                                testCaseResult(ACCEPTED, "", 0)),
+                        testGroupResult(
+                                -1,
+                                testCaseResult(ACCEPTED, "20.0", -1),
+                                testCaseResult(ACCEPTED, "20.0", -1),
+                                testCaseResult(ACCEPTED, "20.0", -1),
+                                testCaseResult(ACCEPTED, "20.0", -1),
+                                testCaseResult(ACCEPTED, "20.0", -1))),
+                List.of(
+                        subtaskResult(-1, ACCEPTED, 100)));
+    }
+
+    @Test
+    void when_solution_received_sigpipe_then_we_should_still_return_the_verdict() throws GradingException {
+        addSourceFile("source", "trigger-solution-sigpipe.cpp");
+        assertResult(
+                new InteractiveGradingConfig.Builder().from(CONFIG)
+                        .communicator("communicator-binary.cpp").build(),
+                ACCEPTED,
+                100,
+                List.of(
+                        testGroupResult(
+                                0,
+                                testCaseResult(ACCEPTED, "[8]", 0),
+                                testCaseResult(ACCEPTED, "[9]", 0),
+                                testCaseResult(ACCEPTED, "[10]", 0)),
+                        testGroupResult(
+                                -1,
+                                testCaseResult(ACCEPTED, "20.0 [9]", -1),
+                                testCaseResult(ACCEPTED, "20.0 [10]", -1),
+                                testCaseResult(ACCEPTED, "20.0 [10]", -1),
+                                testCaseResult(ACCEPTED, "20.0 [9]", -1),
+                                testCaseResult(ACCEPTED, "20.0 [1]", -1))),
+                List.of(
+                        subtaskResult(-1, ACCEPTED, 100)));
+    }
 }

judgels-backends/judgels-grader-engines/src/integTest/resources/engines/interactive/helper/communicator-sigpipe-after-verdict.cpp

@@ -0,0 +1,22 @@
+// This communicator will receive SIGPIPE signal,
+// when paired with trigger-communicator-sigpipe.cpp.
+
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <unistd.h>
+
+int N;
+
+int main(int argc, char* argv[])
+{
+    FILE* in = fopen(argv[1], "r");
+    fscanf(in, "%d", &N);
+
+    fprintf(stderr, "AC\n");
+
+    usleep(500 * 1000);
+
+    printf("this output will trigger SIGPIPE signal");
+    fflush(stdout);
+}

judgels-backends/judgels-grader-engines/src/integTest/resources/engines/interactive/helper/communicator-sigpipe-before-verdict.cpp

@@ -0,0 +1,28 @@
+// This communicator will receive SIGPIPE signal,
+// when paired with trigger-communicator-sigpipe.cpp.
+
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <unistd.h>
+
+int N;
+
+int main(int argc, char* argv[])
+{
+    FILE* in = fopen(argv[1], "r");
+    fscanf(in, "%d", &N);
+
+    usleep(500 * 1000);
+
+    printf("this output will trigger SIGPIPE signal");
+    fflush(stdout);
+
+    // Assume that the communicator never reached the following line,
+    // because it would have been killed by the sandbox.
+
+    // We must comment this out explicitly, because the fake sandbox
+    // used in the tests does not actually sandbox the program.
+
+    // fprintf(stderr, "AC\n");
+}

judgels-backends/judgels-grader-engines/src/integTest/resources/engines/interactive/source/trigger-communicator-sigpipe.cpp

@@ -0,0 +1,4 @@
+// This solution does nothing to trigger the communicator to receive SIGPIPE signal,
+// because this solution would have exited while the communicator is still writing output.
+
+int main() {}

judgels-backends/judgels-grader-engines/src/integTest/resources/engines/interactive/source/trigger-solution-sigpipe.cpp

@@ -0,0 +1,34 @@
+#include <cstdio>
+#include <cstring>
+#include <unistd.h>
+
+char response[100];
+
+int main()
+{
+    int lo = 1, hi = 1000;
+    while (lo <= hi)
+    {
+        int mid = (lo + hi) / 2;
+        printf("%d\n", mid);
+        fflush(stdout);
+
+        fprintf(stderr, "debug");
+        fflush(stderr);
+
+        scanf("%s", response);
+
+        if (!strcmp(response, "too_low"))
+            lo = mid+1;
+        else
+            hi = mid-1;
+    }
+
+    // At this point, the communicator will have exited
+    // with AC verdict.
+
+    usleep(500 * 1000);
+
+    printf("this output will trigger SIGPIPE signal");
+    fflush(stdout);
+}

judgels-backends/judgels-grader-engines/src/main/java/judgels/gabriel/helpers/communicator/Communicator.java

@@ -1,5 +1,9 @@
 package judgels.gabriel.helpers.communicator;
 
+import static judgels.gabriel.api.SandboxExecutionStatus.KILLED_ON_SIGNAL;
+import static judgels.gabriel.api.SandboxExecutionStatus.TIMED_OUT;
+import static judgels.gabriel.api.SandboxExecutionStatus.ZERO_EXIT_CODE;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -19,6 +23,7 @@
 import judgels.gabriel.api.SandboxExecutionStatus;
 import judgels.gabriel.api.SandboxInteractor;
 import judgels.gabriel.api.TestCaseVerdict;
+import judgels.gabriel.api.Verdict;
 import judgels.gabriel.compilers.SingleSourceFileCompiler;
 import judgels.gabriel.helpers.TestCaseVerdictParser;
 import judgels.gabriel.languages.cpp.Cpp17GradingLanguage;
@@ -142,52 +147,60 @@ public EvaluationResult communicate(File input) throws EvaluationException {
         SandboxExecutionResult[] results =
                 sandboxInteractor.interact(solutionSandbox, solutionCommand, communicatorSandbox, command);
 
-        SandboxExecutionResult solutionResult = ignoreSignal13(results[0]);
-        SandboxExecutionResult communicatorResult = ignoreSignal13(results[1]);
-
-        if (communicatorResult.getStatus() != SandboxExecutionStatus.ZERO_EXIT_CODE
-                && communicatorResult.getStatus() != SandboxExecutionStatus.TIMED_OUT) {
-            throw new EvaluationException(String.join(" ", command) + " resulted in " + communicatorResult);
+        SandboxExecutionResult solutionResult = results[0];
+        SandboxExecutionResult communicatorResult = results[1];
+
+        // If the communicator received SIGPIPE, it means it was still writing output while the solution already exited.
+        if (communicatorResult.getExitSignal().equals(Optional.of(13))) {
+            // If the communicator has not written the verdict, it means the solution exited too early.
+            // We return Wrong Answer in this case.
+            if (getCommunicationOutput().isEmpty()) {
+                return new EvaluationResult.Builder()
+                        .verdict(new TestCaseVerdict.Builder().verdict(Verdict.WRONG_ANSWER).build())
+                        .executionResult(solutionResult)
+                        .build();
+            }
         }
 
-        SandboxExecutionResult finalResult;
-        if (communicatorResult.getStatus() == SandboxExecutionStatus.TIMED_OUT) {
-            finalResult = communicatorResult;
-        } else {
-            finalResult = solutionResult;
+        // After we considered the above special case,
+        // we can now safely ignore SIGPIPE from both solution and communicator results.
+        solutionResult = ignoreSignal13(solutionResult);
+        communicatorResult = ignoreSignal13(communicatorResult);
+
+        // If the communicator did not exit successfully, it means there is something wrong with it.
+        if (communicatorResult.getStatus() != ZERO_EXIT_CODE) {
+            throw new EvaluationException(String.join(" ", command) + " resulted in " + communicatorResult);
         }
 
-        TestCaseVerdict verdict;
-
-        Optional<TestCaseVerdict> maybeVerdict = verdictParser.parseExecutionResult(finalResult);
-        if (maybeVerdict.isPresent()) {
-            verdict = maybeVerdict.get();
-        } else {
-            String communicationOutput;
-            try {
-                File communicationOutputFile = communicatorSandbox.getFile(COMMUNICATION_OUTPUT_FILENAME);
-                communicationOutput = FileUtils.readFileToString(communicationOutputFile, StandardCharsets.UTF_8);
-            } catch (IOException e) {
-                throw new EvaluationException(e);
-            }
-            verdict = verdictParser.parseOutput(communicationOutput);
+        Optional<TestCaseVerdict> verdict = verdictParser.parseExecutionResult(solutionResult);
+        if (verdict.isEmpty()) {
+            verdict = Optional.of(verdictParser.parseOutput(getCommunicationOutput()));
         }
 
         communicatorSandbox.removeAllFilesExcept(ImmutableSet.of(communicatorExecutableFilename));
 
         return new EvaluationResult.Builder()
-                .verdict(verdict)
-                .executionResult(finalResult)
+                .verdict(verdict.get())
+                .executionResult(solutionResult)
                 .build();
     }
 
-    // Ignore errors caused by SIGPIPE (broken pipe); treat is as Wrong Answer / Accepted.
+    private String getCommunicationOutput() throws EvaluationException {
+        try {
+            File communicationOutputFile = communicatorSandbox.getFile(COMMUNICATION_OUTPUT_FILENAME);
+            return FileUtils.readFileToString(communicationOutputFile, StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            throw new EvaluationException(e);
+        }
+    }
+
     private static SandboxExecutionResult ignoreSignal13(SandboxExecutionResult result) {
-        if (result.getStatus() == SandboxExecutionStatus.KILLED_ON_SIGNAL
-                && result.getMessage().orElse("").contains("Caught fatal signal 13")) {
+        if (result.getExitSignal().equals(Optional.of(13))) {
             return new SandboxExecutionResult.Builder()
                     .from(result)
-                    .status(SandboxExecutionStatus.ZERO_EXIT_CODE)
+                    .status(ZERO_EXIT_CODE)
+                    .exitSignal(Optional.empty())
+                    .isKilled(false)
                     .message(Optional.empty())
                     .build();
         }

judgels-backends/judgels-grader-engines/src/main/java/judgels/gabriel/sandboxes/fake/FakeSandbox.java

@@ -15,7 +15,6 @@
 
 public class FakeSandbox implements Sandbox {
     private static final int FAKE_TIMED_OUT_EXIT_CODE = 10;
-    private static final int FAKE_KILLED_ON_SIGNAL_EXIT_CODE = 20;
 
     private final File baseDir;
     private File standardInput;
@@ -157,17 +156,13 @@ public SandboxExecutionResult getResult(int exitCode) {
             case FAKE_TIMED_OUT_EXIT_CODE:
                 status = SandboxExecutionStatus.TIMED_OUT;
                 break;
-            case FAKE_KILLED_ON_SIGNAL_EXIT_CODE:
-                status = SandboxExecutionStatus.KILLED_ON_SIGNAL;
-                break;
             default:
                 status = SandboxExecutionStatus.NONZERO_EXIT_CODE;
         }
         return new SandboxExecutionResult.Builder()
                 .time(100)
                 .memory(1000)
                 .status(status)
-                .message("OK")
                 .build();
     }
 }

judgels-backends/judgels-grader-engines/src/main/java/judgels/gabriel/sandboxes/fake/FakeSandboxInteractor.java

@@ -9,6 +9,7 @@
 import judgels.gabriel.api.Sandbox;
 import judgels.gabriel.api.SandboxException;
 import judgels.gabriel.api.SandboxExecutionResult;
+import judgels.gabriel.api.SandboxExecutionStatus;
 import judgels.gabriel.api.SandboxInteractor;
 
 public class FakeSandboxInteractor implements SandboxInteractor {
@@ -43,8 +44,11 @@ public SandboxExecutionResult[] interact(
 
         ExecutorService executor = Executors.newFixedThreadPool(2);
 
-        executor.submit(new UnidirectionalPipe(p1InputStream, p2OutputStream));
-        executor.submit(new UnidirectionalPipe(p2InputStream, p1OutputStream));
+        UnidirectionalPipe pipe1 = new UnidirectionalPipe(p1InputStream, p2OutputStream);
+        UnidirectionalPipe pipe2 = new UnidirectionalPipe(p2InputStream, p1OutputStream);
+
+        executor.submit(pipe1);
+        executor.submit(pipe2);
 
         int exitCode1;
         int exitCode2;
@@ -58,16 +62,35 @@ public SandboxExecutionResult[] interact(
             };
         }
 
-        return new SandboxExecutionResult[]{
-                sandbox1.getResult(exitCode1),
-                sandbox2.getResult(exitCode2)
-        };
+        SandboxExecutionResult result1 = sandbox1.getResult(exitCode1);
+        SandboxExecutionResult result2 = sandbox2.getResult(exitCode2);
+
+        if (pipe1.receivedSignal13) {
+            result1 = newKilledOnSignal13Result(result1);
+        }
+        if (pipe2.receivedSignal13) {
+            result2 = newKilledOnSignal13Result(result2);
+        }
+
+        return new SandboxExecutionResult[]{result1, result2};
+    }
+
+    private static SandboxExecutionResult newKilledOnSignal13Result(SandboxExecutionResult result) {
+        return new SandboxExecutionResult.Builder()
+                .from(result)
+                .status(SandboxExecutionStatus.KILLED_ON_SIGNAL)
+                .exitSignal(13)
+                .isKilled(true)
+                .message("Caught fatal signal 13")
+                .build();
     }
 
     class UnidirectionalPipe implements Runnable {
         private final InputStream in;
         private final OutputStream out;
 
+        boolean receivedSignal13;
+
         UnidirectionalPipe(InputStream in, OutputStream out) {
             this.in = in;
             this.out = out;
@@ -91,7 +114,11 @@ public void run() {
                 out.close();
 
             } catch (IOException e) {
-                throw new SandboxException(e);
+                if (e.getMessage().equals("Stream closed")) {
+                    receivedSignal13 = true;
+                } else {
+                    throw new SandboxException(e);
+                }
             }
         }
     }