Smithay / smithay

A smithy for rusty wayland compositors
MIT License
1.88k stars 164 forks source link

Smithay assumes monotonically increasing serials #161

Closed YaLTeR closed 4 years ago

YaLTeR commented 4 years ago

...however that's not specified anywhere in the docs, and it's not even the case for the default serial counter which is said to wrap around on overflow. Example where the assumption takes place: https://github.com/Smithay/smithay/blob/7fa7fe03be52e35bf7a7b1dbaa947bfe700dfdc6/src/wayland/shell/xdg/xdg_handlers.rs#L320-L325

elinorbgr commented 4 years ago

Indeed, serials should "only" be locally monotonically increasing.

Probably we should add a Serial abstraction that takes the wrapping around into account correctly.

ghost commented 4 years ago

the default serial counter which is said to wrap around on overflow

I'd also add that next_serial() returns a u32 even when usize has 64 bits, so the wrap-around (generated serial is smaller than the previous one) may happen even before that.

Is it reasonable to assume that at most u32::MAX serials have been generated between the ones being compared? If so, it could be solved storing a bool that toggles when the wrap-around happens. Then, if two serials are compared and that flag is different, we can just invert the comparison (e.g. s1 <= s2 instead of s1 > s2.) Because at most u32::MAX serials were generated, the inverted condition should hold.

elinorbgr commented 4 years ago

I had in mind making an implementation of a serial counting taking wrap-around into account based on the fact that serials are overall quite short-lived. So even though a long-lived compositor may wrap-around the u32 in the serials it generates, the actual set of "live" serials should be much closer to each other than that.

So my idea was that if the distance between the two serials is greater than u32::MAX / 2, then assume that a wrap-around occurred between the two.

So, making a Serial(u32) type, with a custom PartialOrd impl taking that into account.

ghost commented 4 years ago

Fair enough. I've made a pull request to do that: https://github.com/Smithay/smithay/pull/230

It's my first time using git to contribute to someone else's project, so please let me know if I screwed up.

elinorbgr commented 4 years ago

Fixed in #230