Project

General

Profile

Actions

Regression #13833

closed

Cron jobs are not removed by ``install_cron_job`` when set inactive as they should be

Added by Jim Pingle almost 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
Configuration Backend
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
23.01
Release Notes:
Force Exclusion
Affected Version:
Affected Architecture:

Description

There is a regression in install_cron_job where it fails to remove cron jobs when they are set inactive ($active == false)

When refactored for PHP 8.1, it's doing an array_slice() on the results of config_get_path() which can't work as it's not going to return a reference which array_splice() expects.

Looks like that can easily be replaced by config_del_path() instead of array_splice().

There are no other instances where that happens in the code, so it's a unique regression.

Actions #1

Updated by Jim Pingle almost 2 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100
Actions #2

Updated by Jim Pingle almost 2 years ago

  • Status changed from Feedback to In Progress

The way the current code locates jobs to alter might not work well with the new way to remove an existing job if multiple operations happen on cron jobs in just the wrong way after removing an existing job.

I'm working on a fix for that, it just needs to identify the index of an existing job in a more accurate way instead of just counting up.

Actions #3

Updated by Jim Pingle almost 2 years ago

Commit is pending for this, but as an example of the problem, consider a scenario similar to the following:

install_cron_job('/root/test1.sh', true, "1", '1', '1', '1', '1', 'root', false);
install_cron_job('/root/test2.sh', true, "2", '2', '2', '2', '2', 'root', false);
install_cron_job('/root/test3.sh', true, "3", '3', '3', '3', '3', 'root', false);
var_dump(config_get_path('cron/item', []));
// Will have all three jobs

// Delete the second job
install_cron_job("/root/test2.sh", false, null, null, null, null, null, null, false);
// Update the third job with new parameters
install_cron_job('/root/test3.sh', true, "30", '3', '3', '3', '3', 'root', false);
var_dump(config_get_path('cron/item', []));

With the old method, the update for test3 after removing test2 would end up with two copies of test3 (one correct at the previous index of test2, one outdated copy of test3 remains at old test3 index). With the new method, there is only the correct updated entry.

Actions #4

Updated by Jim Pingle almost 2 years ago

  • Status changed from In Progress to Feedback
Actions #5

Updated by Jim Pingle almost 2 years ago

  • Status changed from Feedback to Resolved

Current snapshot works correctly all around. Jobs are removed, and subsequent operations in the same batch happen on the correct entries.

Actions

Also available in: Atom PDF