Do not store a FileCacher in the Sandbox.

veluca93 · veluca93 · commit 9f6f2d94e925 · 2026-04-08T10:13:41.000+02:00
diff --git a/cms/grading/Sandbox.py b/cms/grading/Sandbox.py
@@ -198,7 +198,7 @@ class Sandbox:
     def __init__(
         self,
         box_index: int,
-        file_cacher: FileCacher | None,
+        shard: int | None,
         name: str | None = None,
         temp_dir: str | None = None,
     ):
@@ -207,15 +207,13 @@ def __init__(
         box_index: index of this sandbox within the service that wants
             to run it. No two boxes with the same `box_index` should exist
             at the same time.
-        file_cacher: an instance of the FileCacher class
-            (to interact with FS), if the sandbox needs it.
+        shard: the shard index of the service this sandbox runs in, if any.
         name: name of the sandbox, which might appear in the
             path and in system logs.
         temp_dir: temporary directory to use; if None, use the
             default temporary directory specified in the configuration.
 
         """
-        self.file_cacher: FileCacher | None = file_cacher
         self.name: str = name if name is not None else "unnamed"
         self.temp_dir: str = (
             temp_dir if temp_dir is not None else config.global_.temp_dir
@@ -226,12 +224,11 @@ def __init__(
         # the range [0, 1000) for other uses (command-line scripts like cmsMake
         # or direct console users of isolate). Inside each range ids are
         # assigned sequentially, with a wrap-around.
-        if file_cacher is None or file_cacher.service is None:
+        if shard is None:
             box_id = box_index
         else:
             BOXES_PER_SHARD = 1000
             assert box_index < BOXES_PER_SHARD
-            shard = file_cacher.service.shard
             # Note that "shard % 64" might hide misconfiguration.
             # However, since shard numbers are global, there is no good way
             # to have a number in the [0, num_workers_on_this_machine) range.
@@ -510,18 +507,18 @@ def create_file(self, path: str, executable: bool = False) -> typing.BinaryIO:
         return file_
 
     def create_file_from_storage(
-        self, path: str, digest: str, executable: bool = False
+        self, path: str, digest: str, file_cacher: FileCacher, executable: bool = False
     ):
         """Write a file taken from FS in the sandbox.
 
         path: relative path of the file inside the sandbox.
         digest: digest of the file in FS.
+        file_cacher: a FileCacher instance.
         executable: to set permissions.
 
         """
-        assert self.file_cacher is not None
         with self.create_file(path, executable) as dest_fobj:
-            self.file_cacher.get_file_to_fobj(digest, dest_fobj)
+            file_cacher.get_file_to_fobj(digest, dest_fobj)
 
     def create_file_from_string(
         self, path: str, content: bytes, executable: bool = False
@@ -592,21 +589,25 @@ def get_file_to_string(self, path: str, maxlen: int | None = 1024) -> bytes:
                 return file_.read(maxlen)
 
     def get_file_to_storage(
-        self, path: str, description: str = "", trunc_len: int | None = None
+        self,
+        path: str,
+        file_cacher: FileCacher,
+        description: str = "",
+        trunc_len: int | None = None,
     ) -> str:
         """Put a sandbox file in FS and return its digest.
 
         path: relative path of the file inside the sandbox.
+        file_cacher: a FileCacher instance.
         description: the description for FS.
         trunc_len: if None, does nothing; otherwise, before
             returning truncate it at the specified length.
 
         return: the digest of the file.
 
         """
-        assert self.file_cacher is not None
         with self.get_file(path, trunc_len=trunc_len) as file_:
-            return self.file_cacher.put_file_from_fobj(file_, description)
+            return file_cacher.put_file_from_fobj(file_, description)
 
     def stat_file(self, path: str) -> os.stat_result:
         """Return the stats of a file in the sandbox.
diff --git a/cms/grading/steps/trusted.py b/cms/grading/steps/trusted.py
@@ -34,6 +34,7 @@
 "wrong".
 
 """
+from cms.db.filecacher import FileCacher
 
 import logging
 import re
@@ -209,6 +210,7 @@ def trusted_step(
 
 def checker_step(
     sandbox: Sandbox,
+    file_cacher: FileCacher,
     checker_digest: str | None,
     input_digest: str,
     correct_output_digest: str,
@@ -220,6 +222,7 @@ def checker_step(
     sandbox: the sandbox to run the checker in; should already
         contain input, correct output, and user output; the checker is instead
         copied from the managers.
+    file_cacher: the file cacher to use to load the checker.
     checker_digest: digest of the checker, will be fetched as
         "checker"; if None, an appropriate error for bad configuration of the
         task will be generated.
@@ -250,12 +253,12 @@ def checker_step(
         logger.error("Configuration error: missing checker in task managers.")
         return False, None, None, None
     sandbox.create_file_from_storage(CHECKER_FILENAME, checker_digest,
-                                     executable=True)
+                                     file_cacher, executable=True)
 
     # Copy input and correct output in the sandbox.
-    sandbox.create_file_from_storage(CHECKER_INPUT_FILENAME, input_digest)
+    sandbox.create_file_from_storage(CHECKER_INPUT_FILENAME, input_digest, file_cacher)
     sandbox.create_file_from_storage(CHECKER_CORRECT_OUTPUT_FILENAME,
-                                     correct_output_digest)
+                                     correct_output_digest, file_cacher)
 
     # Execute the checker and ensure success, or log an error.
     command = ["./%s" % CHECKER_FILENAME,
diff --git a/cms/grading/tasktypes/Batch.py b/cms/grading/tasktypes/Batch.py
@@ -238,7 +238,7 @@ def _do_compile(self, job, file_cacher):
 
         # Copy required files in the sandbox (includes the grader if present).
         for filename, digest in filenames_and_digests_to_get.items():
-            sandbox.create_file_from_storage(filename, digest)
+            sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         # Run the compilation.
         box_success, compilation_success, text, stats = \
@@ -252,6 +252,7 @@ def _do_compile(self, job, file_cacher):
         if box_success and compilation_success:
             digest = sandbox.get_file_to_storage(
                 executable_filename,
+                file_cacher,
                 "Executable %s for %s" % (executable_filename, job.info))
             job.executables[executable_filename] = \
                 Executable(executable_filename, digest)
@@ -300,9 +301,9 @@ def _execution_step(self, job, file_cacher):
 
         # Put the required files into the sandbox
         for filename, digest in executables_to_get.items():
-            sandbox.create_file_from_storage(filename, digest, executable=True)
+            sandbox.create_file_from_storage(filename, digest, file_cacher, executable=True)
         for filename, digest in files_to_get.items():
-            sandbox.create_file_from_storage(filename, digest)
+            sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         # Actually performs the execution
         box_success, evaluation_success, stats = evaluation_step(
@@ -346,6 +347,7 @@ def _execution_step(self, job, file_cacher):
                 if job.get_output:
                     job.user_output = sandbox.get_file_to_storage(
                         self._actual_output,
+                        file_cacher,
                         "Output file in job %s" % job.info,
                         trunc_len=100 * 1024)
 
diff --git a/cms/grading/tasktypes/Communication.py b/cms/grading/tasktypes/Communication.py
@@ -223,7 +223,7 @@ def compile(self, job, file_cacher):
 
         # Copy all required files in the sandbox.
         for filename, digest in filenames_and_digests_to_get.items():
-            sandbox.create_file_from_storage(filename, digest)
+            sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         # Run the compilation.
         box_success, compilation_success, text, stats = \
@@ -237,6 +237,7 @@ def compile(self, job, file_cacher):
         if box_success and compilation_success:
             digest = sandbox.get_file_to_storage(
                 executable_filename,
+                file_cacher,
                 "Executable %s for %s" % (executable_filename, job.info))
             job.executables[executable_filename] = \
                 Executable(executable_filename, digest)
@@ -282,9 +283,9 @@ def evaluate(self, job, file_cacher):
         sandbox_mgr = create_sandbox(0, file_cacher, name="manager_evaluate")
         job.sandboxes.append(sandbox_mgr.get_root_path())
         sandbox_mgr.create_file_from_storage(
-            self.MANAGER_FILENAME, manager_digest, executable=True)
+            self.MANAGER_FILENAME, manager_digest, file_cacher, executable=True)
         sandbox_mgr.create_file_from_storage(
-            self.INPUT_FILENAME, job.input)
+            self.INPUT_FILENAME, job.input, file_cacher)
 
         # Create the user sandbox(es) and copy the executable.
         sandbox_user = [
@@ -293,7 +294,7 @@ def evaluate(self, job, file_cacher):
         job.sandboxes.extend(s.get_root_path() for s in sandbox_user)
         for i in indices:
             sandbox_user[i].create_file_from_storage(
-                executable_filename, executable_digest, executable=True)
+                executable_filename, executable_digest, file_cacher, executable=True)
 
         # Start the manager. Redirecting to stdin is unnecessary, but for
         # historical reasons the manager can choose to read from there
@@ -425,6 +426,7 @@ def evaluate(self, job, file_cacher):
             if sandbox_mgr.file_exists(self.OUTPUT_FILENAME):
                 job.user_output = sandbox_mgr.get_file_to_storage(
                     self.OUTPUT_FILENAME,
+                    file_cacher,
                     "Output file in job %s" % job.info,
                     trunc_len=100 * 1024)
             else:
diff --git a/cms/grading/tasktypes/TwoSteps.py b/cms/grading/tasktypes/TwoSteps.py
@@ -190,7 +190,7 @@ def compile(self, job, file_cacher):
         job.sandboxes.append(sandbox.get_root_path())
 
         for filename, digest in files_to_get.items():
-            sandbox.create_file_from_storage(filename, digest)
+            sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         # Run the compilation.
         box_success, compilation_success, text, stats = \
@@ -204,6 +204,7 @@ def compile(self, job, file_cacher):
         if box_success and compilation_success:
             digest = sandbox.get_file_to_storage(
                 executable_filename,
+                file_cacher,
                 "Executable %s for %s" %
                 (executable_filename, job.info))
             job.executables[executable_filename] = \
@@ -242,9 +243,10 @@ def evaluate(self, job, file_cacher):
         for filename, digest in first_executables_to_get.items():
             first_sandbox.create_file_from_storage(filename,
                                                    digest,
+                                                   file_cacher,
                                                    executable=True)
         for filename, digest in first_files_to_get.items():
-            first_sandbox.create_file_from_storage(filename, digest)
+            first_sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         first = evaluation_step_before_run(
             first_sandbox,
@@ -265,9 +267,10 @@ def evaluate(self, job, file_cacher):
         for filename, digest in second_executables_to_get.items():
             second_sandbox.create_file_from_storage(filename,
                                                     digest,
+                                                    file_cacher,
                                                     executable=True)
         for filename, digest in second_files_to_get.items():
-            second_sandbox.create_file_from_storage(filename, digest)
+            second_sandbox.create_file_from_storage(filename, digest, file_cacher)
 
         second = evaluation_step_before_run(
             second_sandbox,
@@ -324,6 +327,7 @@ def evaluate(self, job, file_cacher):
                 if job.get_output:
                     job.user_output = second_sandbox.get_file_to_storage(
                         TwoSteps.OUTPUT_FILENAME,
+                        file_cacher,
                         "Output file in job %s" % job.info,
                         trunc_len=100 * 1024)
 
diff --git a/cms/grading/tasktypes/util.py b/cms/grading/tasktypes/util.py
@@ -62,7 +62,8 @@ def create_sandbox(box_index: int, file_cacher: FileCacher, name: str | None = N
 
     """
     try:
-        sandbox = Sandbox(box_index, file_cacher, name=name)
+        shard = file_cacher.service.shard if file_cacher.service is not None else None
+        sandbox = Sandbox(box_index, shard, name=name)
     except OSError:
         err_msg = "Couldn't create sandbox."
         logger.error(err_msg, exc_info=True)
@@ -272,12 +273,13 @@ def eval_output(
             # function, but the type checker isn't smart enough for that
             assert user_output_digest is not None
             sandbox.create_file_from_storage(EVAL_USER_OUTPUT_FILENAME,
-                                             user_output_digest)
+                                             user_output_digest,
+                                             file_cacher)
 
         checker_digest = job.managers[checker_codename].digest \
             if checker_codename in job.managers else None
         success, outcome, text, admin_text = checker_step(
-            sandbox, checker_digest, job.input, job.output,
+            sandbox, file_cacher, checker_digest, job.input, job.output,
             EVAL_USER_OUTPUT_FILENAME, extra_args)
 
         delete_sandbox(sandbox, job, success)
diff --git a/cmstestsuite/unit_tests/grading/steps/fakesandbox.py b/cmstestsuite/unit_tests/grading/steps/fakesandbox.py
@@ -34,8 +34,8 @@ class FakeSandbox(Sandbox):
     answer get_file or get_file_to_string.
 
     """
-    def __init__(self, file_cacher, name=None, temp_dir=None):
-        super().__init__(0, file_cacher, name, temp_dir)
+    def __init__(self, shard, name=None, temp_dir=None):
+        super().__init__(0, shard, name, temp_dir)
         self._fake_files = {}
 
         self._fake_execute_data = deque()
diff --git a/cmstestsuite/unit_tests/grading/steps/trusted_test.py b/cmstestsuite/unit_tests/grading/steps/trusted_test.py
diff --git a/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py b/cmstestsuite/unit_tests/grading/tasktypes/BatchTest.py
diff --git a/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py b/cmstestsuite/unit_tests/grading/tasktypes/CommunicationTest.py
