delete_job_files: inverted active-batch check deletes in-use job files and never reaps finished ones

[FraSanga]

Describe the bug

html/ops/delete_job_files reaps job_file rows (and their download-hierarchy files) once delete_time has passed. Per its own header comment — and job_file.php's — it should delete a file only when it has no associations to active batches. The in-use check is inverted:

// associations to batches in state INIT / IN_PROGRESS (still active)
$query = "select * from DB.batch_file_assoc as bfa join DB.batch
    where bfa.job_file_id=$file->id and bfa.batch_id=batch.id
    and batch.state < BATCH_STATE_COMPLETE";
...
$b = _mysql_fetch_object($result);
if (!$b) continue;        // no active-batch association -> skips deletion
...                       // active-batch association found -> deletes the file

if (!$b) continue; skips deletion when the file is not used by any active batch, and falls through to unlink() + $file->delete() when it is.

Impact

• A job_file past its delete_time that's still referenced by an INIT/IN_PROGRESS batch has its input file deleted from download/ while jobs still need it -> download failures / WU errors.
• A job_file whose batches are all COMPLETE/ABORTED/RETIRED is never reaped -> the file (these are the large shared input files remote submission is meant to support) leaks indefinitely.

The inverted condition has been present since the original 2013 Condor commit (7f4263b07); the 2017 "fix delete_job_files" (#1844) only corrected an unrelated md5->name reference. Blast radius is limited to projects that installed db/schema_condor.sql (so batch_file_assoc exists) and schedule this script themselves — it's in no stock config.xml task list.

Suggested fix

if (!$b) continue; -> if ($b) continue; (an active-batch association should prevent deletion).

While here: unlink($path) leaves the $path.md5 sidecar orphaned, and neither unlink return is checked.

Found while reviewing #6577 (which would build on this daemon).

---

Reported with AI assistance (Claude Sonnet 5); verified against master @ b4b1bad96f.

[FraSanga]

Second bug in the same daemon, on the deletion path itself:

// delete_job_files line 53
BoincBatchFileAssoc::delete_aux("job_file_id=$file->id");

BoincBatchFileAssoc (html/inc/submit_db.inc:142) defines only insert(), lookup(), an instance delete(), and delete_batch() — there is no static delete_aux() and no __callStatic. (delete_aux() is a method on the BoincDb connection object, which the class's own delete()/delete_batch() call as $db->delete_aux(...); it was never a class-level method.) So this line is a fatal Call to undefined method BoincBatchFileAssoc::delete_aux().

It's reached only after unlink($path) and the echo "deleted $path\n" on the two preceding lines, and it precedes $file->delete(). So even where the daemon does run (a project that installed schema_condor.sql and scheduled it), each pass: unlinks the download file, prints "deleted", then fatals before removing the job_file row. On restart $id is back to 0, the same now-file-less row is re-selected, and it fatals again on the same line — the daemon can never remove a job_file DB row or make progress past the first eligible one.

Present since the 2017 "fix delete_job_files" (#1844 / b68e0dafa), which introduced the _mysql_fetch_object refactor but left this call. Combined with the inverted condition above, the daemon deletes in-use files from disk and then immediately crashes without cleaning up the DB.

Fix: replace with a real deletion — either add a BoincBatchFileAssoc class method for "delete all rows for a job_file_id" (mirroring delete_batch()), or inline the query via BoincDb::get()->delete_aux('batch_file_assoc', "job_file_id=$file->id").

---

Reported with AI assistance (Claude Sonnet 5); verified against master @ b4b1bad96f.

[FraSanga]

Correction — I missed an earlier fatal, and it changes the runtime picture.

delete_job_files dies on line 34, before the loop body ever runs. Its first DB call is BoincJobFile::enum(...), but BoincJobFile (html/inc/submit_db.inc:121) defines only insert() / lookup_name() / delete() / update() — no enum(), no parent class, no __callStatic. Same failure as the delete_aux() one, 19 lines earlier. On PHP 8.1+ the script prints its "starting" line and exits at line 34 — it never reaches unlink() (51), echo "deleted" (52), or delete_aux() (53).

What that means for the above:

• The body's Impact — a still-referenced job_file losing its file from download/ — can't happen at runtime; execution never reaches the inverted check on line 48. The inverted if (!$b) continue; is a genuine source bug but latent: it only bites once line 34 is fixed.
• My earlier comment's "unlinks the file, prints 'deleted', then fatals" is wrong — nothing is unlinked; the fatal is line 34, not 53.
• So the daemon has never done more than print one line and exit. Less alarming than "deletes in-use files", but the body's one-line fix is incomplete — BoincJobFile also needs an enum(), or line 34 rewritten as a raw query.

Two history points in my comment were also off: the delete_aux() call dates to the 2013 creation commit (7f4263b0797f), not 2017; and _mysql_fetch_object came from 3abed383e51f (2014, "use mysqli everywhere"), not b68e0dafa (whose entire diff is job_file_name($file->md5) → $file->name). The body's history is correct.

Consolidated: four independent defects — enum() undefined, inverted active-batch check, and delete_aux() undefined all present since 7f4263b0797f, plus unlink() not removing the .md5 sidecar.

---

Verified against master @ 450645283c. Reported with AI assistance (Claude Sonnet 5).