Validate scheduler tasks

[indyteo]

Type of modification

• Code
  • Tools: Scheduler

Issue

MINALAC-114

Changes

Resolves an old TODO and improves scheduler with tasks validation (draft)

Feature description

Spurious wakeup guard
First, what's a "spurious wakeup"?
When a thread is sleeping (because someone called the method .wait() on an object), it can only be awakened by 4 events:

• Someone else (from another thread) calls .notify() / .notifyAll() on the same object;
• The timeout given in the .wait() call expires;
• It is interrupted (for example by the JVM if the invoker hits Ctrl + C);
• It spuriously wakes up (it just happens to wake up, without being notified, timed out or interrupted).

See documentation on this method for details: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Object.html#wait(long,int)

Ok, but why it can just wake up spuriously?
To actually make a thread sleep, a system call is made by the JVM. Then, whenever the system receives a signal, it resumes the program with appropriate flags set (so the JVM knows why the syscall returned). At this point, the JVM can handle special cases, such as SIGINT or SIGTERM. However, if the JVM has nothing special to do for than signal, we could assume it just make a new syscall to wait for the next signal, but this is not the case. In fact, in the short time elapsed since the previous syscall resumed, it could have miss the real signal it was waiting for! So, there's nothing else the JVM can do except from waking up the thread, and boom, it may be a spurious wakeup.

TL;DR: The thread wakes up if the underlying JVM process is signaled.

Note: This is a somewhat simplified explanation and is not 100% accurate, but depicts the issue correctly. More accurate behavior can be found in this StackOverflow answer: https://stackoverflow.com/a/1051816/21276039

And now we know all this, how do we prevent them?
As we saw earlier, there's no real way to prevent a spurious wakeup. The only thing we can do is guard ourselves against them. And the standard way to do so is to wrap our .wait(...) call in a while loop, checking for our real wake-up conditions, including the timeout.

This was a rather long explanation of a rather short patch, interestingly. 😃

Scheduler tasks validation
This is a draft. It's working, but the code is not finalized (documentation, tests...).

[This section is under redaction...]

Showcase

Define a renderer to be executed after an invalid source and you'll get an error.
Create a self dependency (a renderer wanting to execute after itself) and you'll get an error.
Create a cyclic dependency between two renderers and you'll get an error.

Reason

Spurious wakeup guard
In the initial version of the scheduler, the .wait(...) call was not guarded against spurious wakeups. This was just because spurious wakeups do not happen frequently, and the main focus was on having a working product, rather than a perfectly robust one. To avoid accumulating too much TODOs, I decided to resolve this one in the same time I was working on the scheduler, because the fix was fairly simple, after a little bit of thinking.

Scheduler tasks validation
Currently, the generator can get stuck waiting for nonexistent tasks, or cyclic dependencies. While the timeout is here to avoid infinite wait, it would be better to inform the user that the requested parameters are invalid and the generation will fail, without having to wait for the timeout. The validity state of the scheduler can be computed at the beginning from all its scheduled tasks.

TODOs

Removed:

• Scheduler spurious wakeup guard (done).

Self-checks

• The code has unit tests associated
• The code has Javadoc Comments associated
• Complex / Unexpected code is explained / justified with a small comment
• Relevant documentation inside the /docs folder has been updated
• All examples in docs/usage/Examples.md work the same (or have been adapted if subject to changes in this PR)
• Git history is clean (each commit accomplish a single task and describe it accordingly)
• The texts have been proofread (documentation, error messages, logs, comments...)

[indyteo]

Je voudrais un avis sur la logique de validation (parce que je suis parti sur un truc un peu tordu j'ai l'impression), avant d'essayer de mettre au propre le code

[pyrollo]

Dans le configurator, j'avais fait un truc plus bourrin;

    private static class Validator {
        private final Map<String, Set<String>> dependancies = new HashMap<>();

        public Validator(Collection<ScheduledTask> tasks) {
            for (ScheduledTask task : tasks)
                dependancies.put(task.getId(), task.getConditions());
        }

        private void check(List<String> seen, String current) throws IllegalSchedulerException {
            if (seen.contains(current))
                throw new IllegalSchedulerException("Circular dependancy involving task " + current);

            if (!dependancies.keySet().contains(current))
                throw new IllegalSchedulerException("Unknown task " + current);

            List<String> nowSeen = new ArrayList<String>(seen);
            nowSeen.add(current);

            for (String id: dependancies.get(current))
                check(nowSeen, id);
         }

         public void validate() throws IllegalSchedulerException {
            for (String id : dependancies.keySet())
                check(List.of(), id);
        }
    }

Comme le scheduler devrait évoluer, je préssens qu'on va avoir une Map<String, ScheduledTask> tasks au lieu de la liste. Dans ce cas, même plus besoin de la classe Validate, juste les deux méthodes dans la classe Scheduler.