Segfault on decoding of nested lists and maps

[MarshalX]

these benchmarks fail. ref https://github.com/DavidBuchanan314/dag-cbor-benchmark

Decode Torture Tests:
=====================
torture_cids.dagcbor           libipld   75.2 ms (51.98 MB/s)
torture_nested_lists.dagcbor   libipld   SEGFAULT
torture_nested_maps.dagcbor    libipld   SEGFAULT

@DavidBuchanan314 could you pls provide more info about what special about "torture_nested_*"?

[DavidBuchanan314]

They are equivalent to [[[[[[...]]]]]] and {"":{"":{"":{"":{"":{"":...}}}}}} respectively - I believe the parser recurses too deeply and exceeds the native call stack

[MarshalX]

Hi @DavidBuchanan314, do not you want to contribute a fix? :) The depth of recursion is calculated already. The last question is what the limit to set to raise proper error instead of panic?

[DavidBuchanan314]

IDK if it's accessible from the rust side of things, but from the CPython API you can add calls to Py_EnterRecursiveCall and Py_LeaveRecursiveCall, which will allow the runtime to apply the overall sys.getrecursionlimit() limit and raise RecursionError automatically. https://docs.python.org/3/c-api/exceptions.html#recursion-control

But, maybe it would be better to have an explicit limit for CBOR depth, for consistency? If so, around 256 would be pretty conservative

[DavidBuchanan314]

btw if anyone would like test cases, some deeply nested records are still present in the firehose backfill window at https://pds.dev.retr0.id/xrpc/com.atproto.sync.subscribeRepos

[MarshalX]

Yeah. I ve researched it a long while ago and just stopped on putting the emoji on the comment xD PyO3/pyo3#2510 (comment)

As I remember I tried to set hard limit to 256 and it was not enough to typical firehose stream

[DavidBuchanan314]

I'm surprised to hear 256 was not enough, are you sure it wasn't from other people messing around like me?

[MarshalX]

I am not sure. That also was long time ago back to previous winter IG. So I do not remember ;(

[MarshalX]

Ah, that's probably why I stopped

int Py_EnterRecursiveCall(const char *where)
Part of the Stable ABI since version 3.9.

Probably it will be a reason to drop Python 3.8 support. Which died back to beginning of this October