ijl / orjson

Fast, correct Python JSON library supporting dataclasses, datetimes, and numpy
Apache License 2.0
6.06k stars 209 forks source link

Crash reported with 3.10.2+ on aarch64 #495

Closed bdraco closed 3 months ago

bdraco commented 3 months ago

https://github.com/home-assistant/core/issues/118507#issuecomment-2151033933

Thread 1 "hass" received signal SIGBUS, Bus error.
0xf5f6f9f6 in <orjson::serialize::per_type::unicode::StrSerializer as serde::ser::Serialize>::serialize ()
   from target:/usr/local/lib/python3.12/site-packages/orjson/orjson.cpython-312-arm-linux-musleabihf.so
bdraco commented 3 months ago

Maybe related https://github.com/ijl/orjson/commit/d81712f222c768d4313a0d83be04e37c42ce7177

ijl commented 3 months ago

This is likely something to do with arm7 specifically. I removed arm7 as a wheel because it wouldn't pass CI. Why, though? The Rust target is "guaranteed to build", not "guaranteed to work". And this is running nightly, so I don't know. And then the build and test are both emulated in QEMU in an environment I can't reproduce and that take ~15 minutes per iteration. You could imagine doing an entirely separate arm7 stable rust nightly build.

That commit itself is just doing a memcpy instead of check for escapes etc on strings the library constructs itself and are known to not need escaping. There's nothing interesting on 64/32-bit or aarch64/amd64 around then either.

bdraco commented 3 months ago

I thought it was armv7 as well but the linked issue is aarch64 as well

Edit https://github.com/home-assistant/core/issues/118909

And

https://github.com/home-assistant/core/issues/118507#issuecomment-2144756742

bdraco commented 3 months ago

I asked for explicit verification in in linked issue that wasn't explicit and it's now confirmed in both of the linked issues that they are on aarch64

ijl commented 3 months ago

Is your provided setup musl libc and 3.12? orjson-3.10.3-cp312-cp312-musllinux_1_2_aarch64.whl was installed?

bdraco commented 3 months ago

Yes

Musl (alpine 3.19), cpython 3.12, Installed from wheel

ijl commented 2 months ago

3.10.6 has arm7 wheels and should be appropriate to try. It possibly was an issue with the u64 write.

bdraco commented 2 months ago

Thanks for the heads up. I’ll give it a shot when I get home later tonight

bdraco commented 2 months ago

Initial testing looks good.

Will have a better idea if everything is OK in early August when HA core 2024.8.x ships

bdraco commented 1 month ago

So far so good with the HA beta. Will followup again after the stable release.

Thanks

bdraco commented 1 month ago

Home Assistant stable release shipped yesterday and everything seems to be ok. Thanks for the fix!

ijl commented 1 month ago

Thanks for letting me know.