[core] Fix python 3.12 asyncio RecursionError leading to objects_valid check

[dayshah]

Why are these changes needed?

Currently if the task_execution_handler in _raylet.pyx throws an exception before we can get into the actual task handling code that catches all exceptions, cython will end up just continuing execution to return Status::OK. This means that we could finish ExecuteTask with Status::OK and a return objects vector that has empty objects leading to the dreaded objects_valid check failure.

One of the primary reasons this can happen is due to a RecursionError that can be thrown pretty easily (and in an unexpected way to users) due to the way we do asyncio through boost fibers which makes the Python interpreter think our recursion depth is very high. We try to handle this with this code

ray/python/ray/_raylet.pyx

Lines 681 to 701 in 534b0e4

	cdef increase_recursion_limit():
	"""Double the recusion limit if current depth is close to the limit"""
	cdef:
	CPyThreadState * s = <CPyThreadState *> PyThreadState_Get()
	int current_limit = Py_GetRecursionLimit()
	int new_limit = current_limit * 2
	cdef extern from *:
	"""
	#if PY_VERSION_HEX >= 0x30C0000
	#define CURRENT_DEPTH(x) ((x)->py_recursion_limit - (x)->py_recursion_remaining)
	#elif PY_VERSION_HEX >= 0x30B00A4
	#define CURRENT_DEPTH(x) ((x)->recursion_limit - (x)->recursion_remaining)
	#else
	#define CURRENT_DEPTH(x) ((x)->recursion_depth)
	#endif
	"""
	int CURRENT_DEPTH(CPyThreadState *x)
	int current_depth = CURRENT_DEPTH(s)
	if current_limit - current_depth < 500:
	Py_SetRecursionLimit(new_limit)

but it doesn't actually work in the Python 3.12 (PY_VERSION_HEX >= 0x30C0000) case. This is because CPython changed to having separate variables for c recursion and py recursion in 3.12 here python/cpython#96510.

CPython just sets the c_recursion_remaining of the thread state to C_RECURSION_LIMIT at thread state initialization and then only uses c_recursion_remaining, so we can just to increase c_recursion_remaining by 1k once it drops below 1k to keep it over 1k.
https://github.com/python/cpython/blob/bfb9e2f4a4e690099ec2ec53c08b90f4d64fde36/Python/pystate.c#L1353

Repro

Note that there is a test that does something very similar to this and triggers the same condition: test_asyncio.py::test_asyncio_actor_high_concurrency, but it seems like it might not running on Python 3.12 on CI now, trying to get to the bottom of that.

import ray
import asyncio

@ray.remote
class AsyncConcurrencyBatcher:
    def __init__(self, batch_size):
        self.batch = []
        self.event = asyncio.Event()
        self.batch_size = batch_size

    async def add(self, x):
        self.batch.append(x)
        if len(self.batch) >= self.batch_size:
            self.event.set()
        else:
            await self.event.wait()
        return sorted(self.batch)


batch_size = 746
actor = AsyncConcurrencyBatcher.remote(
    batch_size
)
ray.get([actor.add.remote(i) for i in range(batch_size)])

Related issue number

#57173

[jjyao]

Killing boost fiber is on the @edoakes's wishlist

[dayshah]

ya ik I talked to him about it, he said he's gonna try prioritizing next quarter 👀

[jjyao]

This field exists in all Python versions?

[dayshah]

no it doesn't but having it here doesn't cause any issues even if used with old python versions

recursion_limit and recursion_remaining also don't exist for all python versions

[jjyao]

oh, interesting

[jjyao]

What does Do mean here? I thought this function is supposed to be read-only.

[dayshah]

for 3.12 it has to increase c_recursion_remaining now

[jjyao]

Py_SetRecursionLimit is just a C function so it can be called inside cdef extern from *: as well so we can do all the adjustment inside the C function we define? (Just trying to see how to make the code cleaner)

[dayshah]

Oh ya nice suggestion, didn't think of that.

Cleaned up code to just have everything happen in the cdef extern

[jjyao]

Py_SetRecursionLimit is just a C function so it can be called inside cdef extern from *: as well so we can do all the adjustment inside the C function we define? (Just trying to see how to make the code cleaner)

[edoakes]

Good work piecing this together. I think we have enough datapoints to indicate that we really need to get away from boost fiber...