[K9VULN-2502] Decouple timeout watchdog from JsRuntime

[jasonforal]

What problem are you trying to solve?

We will be adding the ability to gracefully recover from a JavaScript execution that causes v8 to run out of memory.

To do this, we need to provide a callback (NearHeapLimitCallback) to v8, which is a fundamentally different model from the condvar/mutex we use for our timeout watchdog.

We should thus decouple the timeout logic from the JsRuntime so that we can handle the increased complexity in an encapsulated way. (But in my opinion, this would be a good refactor irrespective--it's a design smell that our scoped_execute function needs to manually deal with internal implementation details like notifying condvars).

What is your solution?

Remove all timeout logic from the ddsa JsRuntime and introduce a new, more generic "ResourceWatchdog". This will eventually contain the out-of-memory v8 callback implementation.

Note

There is no change to runtime behavior or thread synchronization behavior
This PR is just a structural refactor

Notable Implementation Details

Watchdog state now stores termination reason

struct WatchdogState {
    timeout: TimeoutState,
    /// This will be `Some` if there was a termination. Otherwise, it will be `None`.
    termination_err: Option<DDSAJsRuntimeError>,
}

We need to distinguish between a termination from the v8 callback (out of memory) and our thread (timeout). This requires a mutex to both perform the termination and signal the reason for termination. While channels are probably a better way to encapsulate that communication, I'd like to keep using a Mutex to take advantage of Condvar's wait_timeout api.

We now need to make sure that this WatchdogState is manually cleared after each execution. This is extensively tested with all possible state transition permutations here.

Caller is no longer concerned with implementation details
Calls needing resource limitation just need make a call through the watchdog. Simple:

let execution_result = self
    .v8_resource_watchdog
    .execute(timeout, tc_ctx_scope, |sc| bound_script.run(sc))?;

(See execute function signature here)

Alternatives considered

What the reviewer should know

• For slightly less-intimidating diff, see d1836d2 to see how "JsExecutionState" was simplified.
• spawn_timeout_thread is a copy/paste of the prior implementation with no behavior change.

[juli1]

please review the comment below, fix if you think it's appropriate and ship

[juli1]

why Optional? We should have only DDSAJsRuntimeError. If we do not know the error, then, we have an unknown type that can wrap some error text or error code.

[jasonforal]

It has to be Option<DDSAJsRuntimeError> because after an execution, when we lock the mutex and read the WatchdogState, it's possible that the execution succeeded without any termination at all, so this option will be None.

Otherwise, if there was a termination from a watchdog, it will be Some(DDSAJsRuntimeError::JavaScriptTimeout) (or soon-to-be Some(DDSAJsRuntimeError::JavaScriptMemoryExceeded))

So here, it's not about "known" vs "unknown" error, but rather whether a termination occurred or not.

[jasonforal]

Thanks for reviewing -- rebasing and merging