From a3a9fd56a22754f302c732242f4e1df1120a8ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ron=20Nosz=C3=A1ly?= Date: Mon, 26 Aug 2024 17:57:04 +0200 Subject: [PATCH] Refactor interactive runner to be more robust with malfunctioning interactors and programs --- pkg/language/sandbox/isolate.go | 3 +- pkg/language/sandbox/sandbox.go | 2 +- .../config/problem_yaml/problem_yaml.go | 2 +- pkg/problems/evaluation/run.go | 117 ++++++++++++++---- pkg/problems/evaluation/run_test.go | 56 ++++++++- pkg/problems/evaluation/testdata/empty.py | 0 pkg/problems/evaluation/testdata/error.py | 2 + pkg/problems/evaluation/testdata/printalot.py | 4 + pkg/problems/executable/checker/whitediff.go | 4 +- 9 files changed, 158 insertions(+), 32 deletions(-) create mode 100644 pkg/problems/evaluation/testdata/empty.py create mode 100644 pkg/problems/evaluation/testdata/error.py create mode 100644 pkg/problems/evaluation/testdata/printalot.py diff --git a/pkg/language/sandbox/isolate.go b/pkg/language/sandbox/isolate.go index b4f54c29..a8f0ae29 100644 --- a/pkg/language/sandbox/isolate.go +++ b/pkg/language/sandbox/isolate.go @@ -4,7 +4,6 @@ import ( "bufio" "context" "fmt" - "github.com/mraron/njudge/pkg/language/memory" "io" "log/slog" "os" @@ -13,6 +12,8 @@ import ( "strconv" "strings" "time" + + "github.com/mraron/njudge/pkg/language/memory" ) // IsolateRoot is the root directory structure isolate is using. diff --git a/pkg/language/sandbox/sandbox.go b/pkg/language/sandbox/sandbox.go index 310d74db..cbd20a37 100644 --- a/pkg/language/sandbox/sandbox.go +++ b/pkg/language/sandbox/sandbox.go @@ -147,7 +147,7 @@ type RunConfig struct { // Sandbox is used to Run a command inside a secure sandbox. type Sandbox interface { - Id() string // Id should return an unique ID for sandboxes of the same type + Id() string // Id() should return an unique ID for sandboxes of the same type Init(ctx context.Context) error FS diff --git a/pkg/problems/config/problem_yaml/problem_yaml.go b/pkg/problems/config/problem_yaml/problem_yaml.go index c37140ce..8bccaa7e 100644 --- a/pkg/problems/config/problem_yaml/problem_yaml.go +++ b/pkg/problems/config/problem_yaml/problem_yaml.go @@ -452,7 +452,7 @@ func ParserAndIdentifier(opts ...Option) (problems.ConfigParser, problems.Config *interactorPath = binaryName } - p.Tests.interactorBinary, err = os.ReadFile(*interactorPath) + p.Tests.interactorBinary, err = os.ReadFile(filepath.Join(p.Path, *interactorPath)) if err != nil { return nil, err } diff --git a/pkg/problems/evaluation/run.go b/pkg/problems/evaluation/run.go index 318edb69..f08336ba 100644 --- a/pkg/problems/evaluation/run.go +++ b/pkg/problems/evaluation/run.go @@ -14,6 +14,7 @@ import ( "io" "os" "path/filepath" + "strings" "sync" "syscall" "time" @@ -363,38 +364,81 @@ func (r *InteractiveRunner) getSandboxes(provider sandbox.Provider) (userSandbox return } -func (r *InteractiveRunner) prepareFIFO(dir string, name string) (*os.File, error) { +func (r *InteractiveRunner) prepareFIFO(dir string, name string) (*os.File, *os.File, error) { if err := syscall.Mkfifo(filepath.Join(dir, name), 0666); err != nil { - return nil, err + return nil, nil, err } if err := os.Chmod(filepath.Join(dir, name), 0666); err != nil { - return nil, err + return nil, nil, err } - return os.OpenFile(filepath.Join(dir, name), os.O_RDWR, 0666) + var ( + readFile *os.File + writeFile *os.File + err1, err2 error + ) + var wg sync.WaitGroup + wg.Add(1) + go func() { + readFile, err1 = os.OpenFile(filepath.Join(dir, name), os.O_RDONLY, os.ModeNamedPipe) + wg.Done() + }() + wg.Add(1) + go func() { + writeFile, err2 = os.OpenFile(filepath.Join(dir, name), os.O_WRONLY, os.ModeNamedPipe) + wg.Done() + }() + wg.Wait() + if err1 != nil || err2 != nil { + return nil, nil, errors.Join(err1, err2, readFile.Close(), writeFile.Close()) + } + return readFile, writeFile, nil } type UserInteractorExecutor interface { - ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin, userStdout *os.File, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) - ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin, userStdout *os.File, testcase *problems.Testcase) (*sandbox.Status, error) + ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin io.Reader, userStdout io.Writer, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) + ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin io.Writer, userStdout io.Reader, testcase *problems.Testcase) (*sandbox.Status, error) } type PolygonUserInteractorExecute struct{} -func (p PolygonUserInteractorExecute) ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin, userStdout *os.File, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) { - return language.Run(ctx, userSandbox, userBin, userStdin, userStdout, timeLimit, memoryLimit) +type sandboxWithErrorStream struct { + sandbox.Sandbox + ErrorStream io.Writer } -func (p PolygonUserInteractorExecute) ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin, userStdout *os.File, testcase *problems.Testcase) (*sandbox.Status, error) { - return interactorSandbox.Run(ctx, sandbox.RunConfig{ +func (s sandboxWithErrorStream) Run(ctx context.Context, config sandbox.RunConfig, toRun string, toRunArgs ...string) (*sandbox.Status, error) { + config.Stderr = s.ErrorStream + return s.Sandbox.Run(ctx, config, toRun, toRunArgs...) +} + +func (p PolygonUserInteractorExecute) ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin io.Reader, userStdout io.Writer, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) { + return language.Run(ctx, sandboxWithErrorStream{ + userSandbox, + os.Stderr, + }, userBin, userStdin, userStdout, timeLimit, memoryLimit) +} + +func (p PolygonUserInteractorExecute) ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin io.Writer, userStdout io.Reader, testcase *problems.Testcase) (*sandbox.Status, error) { + interactorOutput := &bytes.Buffer{} + res, err := interactorSandbox.Run(ctx, sandbox.RunConfig{ RunID: "interactor", TimeLimit: 2 * testcase.TimeLimit, MemoryLimit: 1 * memory.GiB, Stdin: userStdout, Stdout: userStdin, - Stderr: io.Discard, + Stderr: interactorOutput, InheritEnv: true, WorkingDirectory: interactorSandbox.Pwd(), }, "interactor", "input.txt", "output.txt") + if err != nil { + return res, err + } + if strings.Contains(interactorOutput.String(), "FAIL") { + res.Verdict = sandbox.VerdictXX + } else if res.Verdict == sandbox.VerdictRE && strings.Contains(interactorOutput.String(), "wrong answer") { + res.Verdict = sandbox.VerdictOK + } + return res, nil } type interactorOutput struct { @@ -435,11 +479,11 @@ func (t *TaskYAMLUserInteractorExecute) Check(ctx context.Context, testcase *pro return nil } -func (t *TaskYAMLUserInteractorExecute) ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin, userStdout *os.File, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) { +func (t *TaskYAMLUserInteractorExecute) ExecuteUser(ctx context.Context, userSandbox sandbox.Sandbox, language language.Language, userBin sandbox.File, userStdin io.Reader, userStdout io.Writer, timeLimit time.Duration, memoryLimit memory.Amount) (*sandbox.Status, error) { return language.Run(ctx, userSandbox, userBin, userStdin, userStdout, timeLimit, memoryLimit) } -func (t *TaskYAMLUserInteractorExecute) ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin, userStdout *os.File, testcase *problems.Testcase) (*sandbox.Status, error) { +func (t *TaskYAMLUserInteractorExecute) ExecuteInteractor(ctx context.Context, interactorSandbox sandbox.Sandbox, userStdin io.Writer, userStdout io.Reader, testcase *problems.Testcase) (*sandbox.Status, error) { inputFile, err := os.Open(filepath.Join(interactorSandbox.Pwd(), "input.txt")) if err != nil { return nil, err @@ -450,6 +494,9 @@ func (t *TaskYAMLUserInteractorExecute) ExecuteInteractor(ctx context.Context, i res.scoreMul = &bytes.Buffer{} t.forChecker.Store(testcase.Index, res) + userStdoutName := (userStdout).(*os.File).Name() + userStdinName := (userStdin).(*os.File).Name() + return interactorSandbox.Run(ctx, sandbox.RunConfig{ RunID: "interactor", TimeLimit: 2 * testcase.TimeLimit, @@ -468,7 +515,7 @@ func (t *TaskYAMLUserInteractorExecute) ExecuteInteractor(ctx context.Context, i }, }, }, - }, "interactor", userStdout.Name(), userStdin.Name()) + }, "interactor", userStdoutName, userStdinName) } func (r *InteractiveRunner) Run(ctx context.Context, sandboxProvider sandbox.Provider, testcase *problems.Testcase) error { @@ -522,20 +569,22 @@ func (r *InteractiveRunner) Run(ctx context.Context, sandboxProvider sandbox.Pro return err } - userStdin, err := r.prepareFIFO(dir, "fifo1") + userStdinRead, userStdinWrite, err := r.prepareFIFO(dir, "fifo1") if err != nil { return err } - defer func(userStdin *os.File) { - _ = userStdin.Close() - }(userStdin) - userStdout, err := r.prepareFIFO(dir, "fifo2") + defer func(userStdinRead *os.File, userStdinWrite *os.File) { + _ = userStdinRead.Close() + _ = userStdinWrite.Close() + }(userStdinRead, userStdinWrite) + userStdoutRead, userStdoutWrite, err := r.prepareFIFO(dir, "fifo2") if err != nil { return err } - defer func(userStdout *os.File) { - _ = userStdout.Close() - }(userStdout) + defer func(userStdoutRead *os.File, userStdoutWrite *os.File) { + _ = userStdoutRead.Close() + _ = userStdoutWrite.Close() + }(userStdoutRead, userStdoutWrite) var ( userStatus, interactorStatus *sandbox.Status @@ -547,11 +596,31 @@ func (r *InteractiveRunner) Run(ctx context.Context, sandboxProvider sandbox.Pro userStatus, userError = r.executor.ExecuteUser(ctx, userSandbox, r.lang, sandbox.File{ Name: r.userBinName, Source: io.NopCloser(bytes.NewBuffer(r.userBin)), - }, userStdin, userStdout, testcase.TimeLimit, testcase.MemoryLimit) + }, userStdinRead, userStdoutWrite, testcase.TimeLimit, testcase.MemoryLimit) + _ = userStdoutWrite.Close() + + buf := make([]byte, 8*1024) + for { + _, err := userStdinRead.Read(buf) + if err != nil { + break + } + } + _ = userStdinRead.Close() + done <- struct{}{} }() - interactorStatus, interactorError = r.executor.ExecuteInteractor(ctx, interactorSandbox, userStdin, userStdout, testcase) + interactorStatus, interactorError = r.executor.ExecuteInteractor(ctx, interactorSandbox, userStdinWrite, userStdoutRead, testcase) + _ = userStdinWrite.Close() + buf := make([]byte, 8*1024) + for { + _, err := userStdoutRead.Read(buf) + if err != nil { + break + } + } + _ = userStdoutRead.Close() <-done testcase.OutputPath = filepath.Join(interactorSandbox.Pwd(), "output.txt") diff --git a/pkg/problems/evaluation/run_test.go b/pkg/problems/evaluation/run_test.go index a0cf6d3c..fa1a6129 100644 --- a/pkg/problems/evaluation/run_test.go +++ b/pkg/problems/evaluation/run_test.go @@ -5,6 +5,10 @@ import ( "bytes" "context" "fmt" + "os" + "testing" + "time" + "github.com/mraron/njudge/pkg/internal/testutils" "github.com/mraron/njudge/pkg/language/langs/cpp" "github.com/mraron/njudge/pkg/language/langs/python3" @@ -15,9 +19,6 @@ import ( "github.com/mraron/njudge/pkg/problems/executable/checker" "github.com/spf13/afero" "github.com/stretchr/testify/assert" - "os" - "testing" - "time" ) func TestBasicRunner_Run(t *testing.T) { @@ -334,6 +335,7 @@ func TestInteractiveRunner_Run(t *testing.T) { fs := afero.NewMemMapFs() assert.NoError(t, afero.WriteFile(fs, "input", []byte("11 12\n"), 0644)) assert.NoError(t, afero.WriteFile(fs, "answer", []byte("23\n"), 0644)) + assert.NoError(t, afero.WriteFile(fs, "empty", []byte("\n"), 0644)) assert.NoError(t, afero.WriteFile(fs, "input_multi", []byte("1\n11 12\n"), 0644)) assert.NoError(t, afero.WriteFile(fs, "manager.cpp", mustReadFile("testdata/taskyaml_manager.cpp"), 0644)) @@ -420,6 +422,54 @@ func TestInteractiveRunner_Run(t *testing.T) { wantVerdict: problems.VerdictAC, wantScore: 10.0, }, + { + name: "printalot_polygon", + solution: evaluation.NewByteSolution(python3.Python3{}, mustReadFile("testdata/empty.py")), + ir: evaluation.NewInteractiveRunner( + mustReadFile("testdata/printalot.py"), + checker.NewWhitediff(checker.WhiteDiffWithFs(fs, afero.NewOsFs())), + evaluation.InteractiveRunnerWithFs(fs), + ), + args: args{ + ctx: context.TODO(), + sandboxProvider: sandbox.NewProvider().Put(s1).Put(s2), + testcase: &problems.Testcase{ + Index: 1, + InputPath: "input", + OutputPath: "output", + AnswerPath: "empty", + MaxScore: 10.0, + TimeLimit: 1 * time.Second, + }, + }, + wantErr: assert.NoError, + wantVerdict: problems.VerdictAC, + wantScore: 10.0, + }, + { + name: "interactor_error", + solution: evaluation.NewByteSolution(python3.Python3{}, mustReadFile("testdata/empty.py")), + ir: evaluation.NewInteractiveRunner( + mustReadFile("testdata/error.py"), + checker.NewWhitediff(checker.WhiteDiffWithFs(fs, afero.NewOsFs())), + evaluation.InteractiveRunnerWithFs(fs), + ), + args: args{ + ctx: context.TODO(), + sandboxProvider: sandbox.NewProvider().Put(s1).Put(s2), + testcase: &problems.Testcase{ + Index: 1, + InputPath: "input", + OutputPath: "output", + AnswerPath: "empty", + MaxScore: 10.0, + TimeLimit: 1 * time.Second, + }, + }, + wantErr: assert.Error, + wantVerdict: problems.VerdictXX, + wantScore: 0.0, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/problems/evaluation/testdata/empty.py b/pkg/problems/evaluation/testdata/empty.py new file mode 100644 index 00000000..e69de29b diff --git a/pkg/problems/evaluation/testdata/error.py b/pkg/problems/evaluation/testdata/error.py new file mode 100644 index 00000000..a7adf4b0 --- /dev/null +++ b/pkg/problems/evaluation/testdata/error.py @@ -0,0 +1,2 @@ +import sys +sys.exit(123) \ No newline at end of file diff --git a/pkg/problems/evaluation/testdata/printalot.py b/pkg/problems/evaluation/testdata/printalot.py new file mode 100644 index 00000000..d46ce604 --- /dev/null +++ b/pkg/problems/evaluation/testdata/printalot.py @@ -0,0 +1,4 @@ +#!/usr/bin/python3 +print(5*10**6*'a') +with open("output.txt", "w") as w: + w.write('') \ No newline at end of file diff --git a/pkg/problems/executable/checker/whitediff.go b/pkg/problems/executable/checker/whitediff.go index f64a03c3..03a0c6c3 100644 --- a/pkg/problems/executable/checker/whitediff.go +++ b/pkg/problems/executable/checker/whitediff.go @@ -49,7 +49,7 @@ func (w Whitediff) Check(ctx context.Context, testcase *problems.Testcase) error ans, err := w.answerFs.Open(tc.AnswerPath) if err != nil { - return errors.Join(err, ans.Close()) + return err } defer func(ans afero.File) { _ = ans.Close() @@ -57,7 +57,7 @@ func (w Whitediff) Check(ctx context.Context, testcase *problems.Testcase) error out, err := w.outputFs.Open(tc.OutputPath) if err != nil { - return errors.Join(err, out.Close()) + return errors.Join(err, ans.Close()) } defer func(out afero.File) { _ = out.Close()