revery-ui / revery

:zap: Native, high-performance, cross-platform desktop apps - built with Reason!
https://www.outrunlabs.com/revery/
MIT License
8.06k stars 196 forks source link

fix(sdl2): Fix FFI for Display* functions #1026

Closed bryphe closed 3 years ago

bryphe commented 3 years ago

Issue: I had observed a new, intermittent crash in Onivim 2 when running on Windows. It was a pretty obscure crash, because it only reproduced with esy run -f, and not with esy run or esy run -f --trace. This suggested it was likely a GC / FFI issue.

After spending time bisecting, and checking new changes to our C FFI, I realized it went fairly far back... and I had just added a second monitor to my Windows machine. I realized if I unhooked the new monitor, it no longer reproduced. I suspected it may have to do with the FFI relating to the SDL_GetDisplay* functions - so I took a pass over it, and realized we were missing some Val_ints when passing the display width, height, and refresh rate.

This seems innocuous, but without a proper tag bit (https://dev.realworldocaml.org/runtime-memory-layout.html), these values would be treated as pointers by the garbage collector, rather than integers.

Fix: Use Val_int when sending these values back, so they are treated properly by the garbage collector.