Handle CPU limit exceeded in Python workers

[hoodmane]

If we call TerminateExecution(), it will exit Python execution without unwinding the stack or cleaning up the runtime state. This leaves the Python runtime in a permanently messed up state and all further requests will fail.

This adds a new cpuLimitNearlyExceededCallback to the limit enforcer and hooks it up so that it triggers a SIGINT inside of Python. This can be used to raise a CpuLimitExceeded Python error into the runtime. If this error is ignored, then we'll hit the hard limit and be terminated. If we ever do call TerminateExecution() on a Python isolate, we should condemn the isolate, but that is left as a TODO.

To trigger the SIGINT inside of Python, we have to set two addresses:

1. we set emscripten_signal_clock to 0 to make the Python eval breaker check for a signal on the next tick.
2. we set _Py_EMSCRIPTEN_SIGNAL_HANDLING to 1 to make Python check the signal clock.

We also have to set Module.Py_EmscriptenSignalBuffer to a buffer with the number of the signal we wish to trip in it (SIGINT aka 2).

When we start a request we set _Py_EMSCRIPTEN_SIGNAL_HANDLING to 0 to avoid ongoing costs of calling out to JavaScript to check the buffer when no signal is set, and we put a 2 into Py_EmscriptenSignalBuffer.

The most annoying aspect of this is that the symbol emscripten_signal_clock is not exported. For Pyodide 0.28.2, I manually located the address of this symbol and hard coded it. For the next Pyodide, we'll make sure to export it.

[guybedford]

It would be really useful to have this available to Rust as well - could we expose this under a cloudflare:wasm/internal or similar?

[hoodmane]

Seems to me that Rust would also use Worker::Isolate::from(js).setCpuLimitNearlyExceededCallback() but we'll need different C++ logic for it.

[codspeed-hq]

CodSpeed Performance Report

Merging #5543 will not alter performance

_{Comparing hoodmane/python-cpu-limiter (49ec0b1) with main (1a26e6a)}

Summary

✅ 57 untouched
⏩ 30 skipped¹

Footnotes

1. 30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[github-actions]

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

[danlapid]

How do we make sure that this only terminates the current python request and not the entire python runtime?

[hoodmane]

Every time we enter Python code we call setupRequestSignalHandling() which clears the interrupt request. So if we exit from Python exactly as we requested the cpu interrupt, then we'll clear it as we call back in.

[dom96]

Could rapid requests cause the cpu interrupt to be cleared before it has a chance to be processed?

[hoodmane]

As far as I understand, the isolate only can be working on one request at a time, so as long as we clear the request for an interrupt when we enter a request we should be okay.

However, there is a potential problem that we don't clear the interrupt request when entering an already scheduled async task. I will fix that.

[dom96]

Hmm, I don't think it's true that the isolate can only be working on one request at a time. Imagine a request waiting many seconds on some IO, an isolate should remain responsive to other requests during that time, so it should be possible for it to respond to other requests even if others are currently in progress. I think you should add a test for this case and ensure it works as expected.

[jasnell]

It's a bit nuanced. Only a single request will hold the isolate lock at any given time, so in that sense it's true. While a request is waiting on i/o, it will release the lock so that other requests can use it. But keep in mind that we have the microtask queue, which is always global to the isolate. When the task queue gets drained during one request, it can include tasks from other requests. If those aren't scheduling i/o, then the work from those other requests are progressing with this request. This is why we needed the cross-request promise signaling. That arranges it so that promise continuations happen under the correct iocontext, but there are still things like queueMicrotask() which may still end up crossing over and running under a different IoContext.

[guybedford]

So for async requests, failing the promise with the limit exceeded error is necessary as opposed to a synchronous failure?

Or is it okay for async requests to possibly have the limit exceeded trigger for a request that was not itself cpu heavy if other cpu work is being done at the time?

[hoodmane]

I have it set up to clear the signal request every time we enter Python, regardless of whether it is the same request or a different one. So this means that if we exit Python just as the CPU limit triggers we will always clear the signal. If the request keeps going asynchronously, this may mean we hit the hard limit instead. But I think the chance that it exits exactly at the right time to trigger this behavior is very low.