Unsoundness in crate::linux::common::Display's Drop impl

[sersorrel]

rdev::linux::common::Display implements the Drop trait to automatically call XCloseDisplay() when a Display is dropped:

https://github.com/Narsil/rdev/blob/42c00f06b51c2874557a67f66e23f2e906736356/src/linux/common.rs#L128-L134

this is very convenient! unfortunately, it is not sound – XCloseDisplay() is not thread-safe, unless XInitThreads() was called when the program started (and rdev does not do this as far as I can tell), so calling it in multiple threads at once is not a good idea. Whilst rdev's Display is not Send or Sync (since it contains a *mut xlib::Display), it's entirely possible for safe code to just create and drop a Display in multiple threads at once, perhaps by using rdev::display_size():

https://github.com/Narsil/rdev/blob/42c00f06b51c2874557a67f66e23f2e906736356/src/linux/display.rs#L4-L7

[Narsil]

Oh ! Nice catch.

Sorry for the late reply, for some reason I wasn't notified of issues in my own repo ???

Any ideas on how to fix this ? What exactly will go on if you close display (which you opened in the same thread) ?

[sersorrel]

I'm not sure of the best way to fix it, unfortunately. When I opened this issue I was thinking about writing my own safe X11 wrapper library (which would have to deal with stuff like this), but I've mostly forgotten how I was planning to deal with thread-safety stuff like this.

You can find people online who use Xlib in multiple threads simultaneously and encounter crashes/segfaults/other weird behaviour. It seems pretty unlikely that you'd run into this accidentally with rdev, though, since Display isn't public as far as I can tell – it's just something I noticed when looking at how display_size works, I think, I don't think it ever actually caused me any problems.