Skip to content

Commit

Permalink
Countermeasures against a race condition bug
Browse files Browse the repository at this point in the history
  • Loading branch information
toresbe committed Jul 24, 2023
1 parent 3bfa344 commit 709c45d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/scheduling/ScheduleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class ScheduleLoader {
load = async (entries: ScheduleEntry[]) => {
log.info("Loading schedule");

if (timeline.getEvents().length) await timeline.clear();
if (timeline.getEvents().length) timeline.clear();

let prevEnds: Date | undefined;

Expand Down
28 changes: 26 additions & 2 deletions src/scheduling/Timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,33 @@ export class Timeline {
this.events.sort((a, b) => a.fireAt.getTime() - b.fireAt.getTime());
};

clear = async () => {
clear = () => {
log.info("Clearing timeline jobs");
await Promise.all(this.events.map(({ job }) => job.cancel()));
this.events.forEach(({ job }, index) => {
// Here a race condition-like problem was encountered which crashed the playout.
// One failed assumption was that the job.cancel() method would be asynchronous,
// so I iterated over them with Promise.all().
//
// However, an exception arose that job == null, which was unexpected.
//
// The logic has been changed to iterate over the jobs synchronously, and
// both the unexpected and expected cases are logged for more context
// should the issue arise again.
if (!job?.cancel) {
log.warn(
`Job ${index + 1}/${this.events.length} ` +
`(${job?.name}) has no cancel method`,
);
return;
}

log.info(
`Cancelling job ${index + 1}/${this.events.length} (${job?.name})`,
);

job.cancel();
});

this.events = [];
};

Expand Down

0 comments on commit 709c45d

Please sign in to comment.