pythonize icon indicating copy to clipboard operation
pythonize copied to clipboard

x86 Segmentation fault on depythonize

Open keats720 opened this issue 11 months ago • 2 comments
trafficstars

Ran into an issue with x86 where heavily nested data was causing a segmentation fault on depythonize.

Issue came from using sqloxide to get ast for queries, narrowed it down to be happening in depythonize. The implementation works with no issue on Arm architecture. Not familiar with rust so I am not sure exactly how to get a full stack trace on the error, but thought it would be good to try and give some information.

The python dictionary has a depth close to 1000, it is an AST of a query with ~800 unions and is represented by rust sqlparser crate as {left: <>, right: {left: <>, right: ...}}.

I am not sure if it is something small or a complex issue, happy to provide more information if you can guide me on how

keats720 avatar Dec 02 '24 18:12 keats720

You are right about depth. The deserialization is recursive, and there is no control of the depth at all. You get a segmentation fault because of stack overflow. The limit depends on the machine configuration. That's why it could pass in the environment with an ARM arch.

Let's take a look at map deserialization: https://github.com/davidhewitt/pythonize/blob/4491cdb46f35ab3fc1b419beef92171fbef510da/src/de.rs#L286-L291

As we can see there is no depth counting or calling Py_EnterRecursiveCall + Py_LeaveRecursiveCall.

Here is the example of how serde json fixed the same issue: https://github.com/serde-rs/json/pull/163/files (tl;dr count remaining_depth; hard limit to 128; if reach return error; do -1 before visit_map and +1 after visit_map)

Speaking of the Py_EnterRecursiveCall + Py_LeaveRecursiveCall I used it in the exact same bug report in my lib. This is the best and smoothest option for the devs, but, from my testing, it gives 30% perf degradation. You can check it here: https://github.com/MarshalX/python-libipld/pull/51

@davidhewitt I will be happy to read your thoughts!

MarshalX avatar Dec 02 '24 19:12 MarshalX

upd. I decline to use Py_EnterRecursiveCall + Py_LeaveRecursiveCall because it is slow. I choose to count depth by myself and check it against Py_GetRecursionLimit to raise PyRecursionError

MarshalX avatar Mar 03 '25 10:03 MarshalX