fix issue #1732

cms/grading/tasktypes/interactive_keeper.py

@@ -22,6 +22,7 @@
 import os
 import shutil
 import sys
+import time
 from functools import reduce
 
 from cms.grading.Sandbox import Sandbox, wait_without_std
@@ -97,6 +98,10 @@ def main():
             with open(os.path.join(temp_dir, path), "rb") as g:
                 shutil.copyfileobj(g, f)
 
+    # Solutions must finish at least 1s before the controller's wall limit, so
+    # that the controller always outlives them and can report an outcome: any
+    # controller failure is thus a problem with the task.
+    solutions_deadline = time.monotonic() + controller_wall_limit - 1
     controller_proc = evaluation_step_before_run(
         controller_sandbox,
         controller_command,
@@ -144,17 +149,15 @@ def main():
             # Matches Communication's handling of multi-command executions.
             if len(solution_commands) > 1:
                 trusted_step(sandbox_sol, solution_commands[:-1])
+            # Same default wall limit as Batch, capped by the deadline.
+            wall_limit = solutions_deadline - time.monotonic()
+            if solution_time_limit is not None:
+                wall_limit = min(wall_limit, 2 * solution_time_limit + 1)
             sol_proc = evaluation_step_before_run(
                 sandbox_sol,
                 solution_commands[-1],
                 time_limit=solution_time_limit,
-                wall_limit=controller_wall_limit,
-                # the wall-clock limit mostly exists to eventually kill stuck
-                # solutions. there's no point setting it higher than the
-                # controller's limit (as then the controller would die before
-                # any of the solutions), and there's little to be gained from
-                # setting it any lower. to reduce the amount of unnecessary
-                # configuration, set it to the same limit as the controller.
+                wall_limit=max(wall_limit, 0.001),  # must be positive
                 memory_limit=solution_memory_limit,
                 stdin_redirect=p["c_to_u"][0],
                 stdout_redirect=p["u_to_c"][1],
@@ -234,12 +237,15 @@ def do_merge(a, b):
     except Exception as e:
         success = False
         text = ["Internal error"]
-        admin_text = [f"Internal error: {e}"]
+        admin_text = f"Internal error: {e}"
 
     if not success:
         pass
     elif not (success_mgr and box_success_user):
         success = False
+    elif not evaluation_success_mgr:
+        success = False
+        text = ["Controller failed"]
     else:
         if not evaluation_success_user:
             outcome = 0.0
@@ -250,11 +256,11 @@ def do_merge(a, b):
                     + controller_text[1:]
                     + text[1:]
                 )
-        elif not evaluation_success_mgr:
-            outcome = 0.0
-            text = ["Controller failed"] + (controller_text if controller_text else [])
+        elif score is None:
+            success = False
+            text = ["Controller did not report a score"]
         else:
-            outcome = score if score is not None else 0.0
+            outcome = score
             text = controller_text
 
     result = {

cmscontrib/loaders/italy_yaml.py

@@ -790,6 +790,20 @@ def get_task(self, get_statement=True) -> Task | None:
             controller_time_limit = conf.get("controller_time_limit", None)
             controller_wall_limit = conf.get("controller_wall_time_limit", None)
 
+            if controller_memory_limit_mb is None or controller_wall_limit is None:
+                logger.critical("Interactive tasks require controller_memory_limit "
+                                "and controller_wall_time_limit")
+                sys.exit(1)
+
+            # Solutions get a wall limit of 2*time_limit+1, but must finish
+            # 1s before the controller's wall limit.
+            time_limit = args.get("time_limit")
+            if time_limit is not None and controller_wall_limit <= 2 * time_limit + 2:
+                logger.warning("controller_wall_time_limit (%s) is not larger than "
+                               "2*time_limit+2 (%s): solutions' wall time limit "
+                               "will be reduced", controller_wall_limit,
+                               2 * time_limit + 2)
+
             args["task_type_parameters"] = \
                 [process_limit, compilation_param, concurrent,
                  controller_memory_limit_mb, controller_time_limit, controller_wall_limit]

docs/Task types.rst

@@ -135,6 +135,8 @@ Interactive has six parameters:
 - the maximum CPU time (in seconds) that the controller can use;
 - the maximum wall time (in seconds) that the controller can use.
 
+Each solution gets the same wall time limit as in Batch, reduced if needed so that all solutions terminate at least one second before the controller's wall time limit. Hence, the controller should always be able to report an outcome: any controller failure is reported as an evaluation failure, as it indicates a problem with the task.
+
 The ``concurrent`` parameter controls how the total wall clock time and total memory usage of the submission is computed. If concurrent, then wall clock times are max'd and memory usages added; if not concurrent, then wall clock times are added and memory usages max'd. This only affects the display of statistics, not the actual verdict (for the verdict, each spawned solution gets the full time/memory limit). Also note that CPU times are always added.
 
 Note that in the current implementation, all file descriptors for communication are preallocated before running the controller, which means that the controller always uses (at least) ``2*process_limit`` file descriptors. Thus it is not a good idea to set process_limit significantly higher than necessary.