Switch all time values in X11 to UIntPtr#20864
Switch all time values in X11 to UIntPtr#20864ThadHouse wants to merge 2 commits intoAvaloniaUI:masterfrom
Conversation
They're all unsigned at the native layer. This means we should always be treated them as unsigned in the FFI layer
There was a problem hiding this comment.
This just moves the problem by 25 days into the future. For 32 bit platforms we need to track timestamp values and detect overflows. When overflow occurs the tracker should increase overflow counter and then add uint.MaxValue*overflowCount to the timestamp. That way RawPointerEventArgs (where timestamp is always 64 bit ulong) would receive a monotonically incrementing timestamp value
|
It doesn’t completely move the problem by 25 days. Like I said in the description, yes rollover to 0 is a problem, but this pr was only meant to solve the actual crash caused by overflowing to a massive positive number. Tracking rollovers might not be possible, because it’s possible to not get an event for 50 days. I think since that’s technically a different problem, it should be a different PR. |
They're all unsigned at the native layer. This means we should always be treated them as unsigned in the FFI layer
What does the pull request do?
#20858 reported a bug where on 32 bit linux, the signed IntPtr would overflow after 25 days. This meant when converting to long and then to ulong, the negative value would overflow to a massively large positive value, causing a crash when converting that value to a TimeSpan.
The PR switches all instances of time in X11 from IntPtr to UIntPtr. This means theres no signed expansion, which means the cast on 32 bit platforms from UIntPtr to ulong will just rollover back to 0, instead of a huge negative number.
On 64 bit platforms, this change is unobservable, as getting to 63 bits of milliseconds is well beyond any feasable timespan.
What is the current behavior?
An event occuring after 25 days on 32 bit platforms rolls over to a very large value and crashes.
What is the updated/expected behavior with this PR?
The event timestamps last for 50 days, but then rollover to 0, instead of a large positive number.
Breaking changes
None, all internal structures.
Fixed issues
Fixes the crash of ##20858, but doesn't check what the behavior of other parts of the system is after rollover.
-->