Thread safety?

[Philipp91]

See also here:

You can mark your struct Send if the C code dereferencing the pointer never uses thread-local storage or thread-local locking. This happens to be true for many libraries.

You can mark your struct Sync if all C code able to dereference the pointer always dereferences in a thread-safe manner, i.e. consistent with safe Rust. Most libraries that obey this rule will tell you so in the documentation, and they internally guard every library call with a mutex.

---

I think unsafe impl Send for XDo {} can be included in this library, right? AFAIU this would allow putting it in a Mutex<>.

I'm not so sure about unsafe impl Sync for XDo {}. @jordansissel Can you tell whether the xto_t* returned from xdo.h is thread-safe? Given that it's mostly consumed as const (except for the xdo_free function). AFAIU this would allow dropping the Mutex<> again.

---

My goal: Put libxdo::XDo into a struct that must be Send and Sync, and ideally not use sth like Mutex<> that can block calling threads. That is, if the underlying C library is fine with that, I'd like to allocate an instance once and then call various functions on it possibly concurrently.

[jordansissel]

I haven’t done any extensive analysis of xdo’s thread safety, but I think the library generally should be safe this way.

IIRC, the struct itself is assumed private (while C doesn’t provide this concept). The only part I recall doing any modification of the xdo struct is scanning the key map, and that happens only on initialization here: https://github.com/jordansissel/xdotool/blob/master/xdo.c#L134

Best guess, this library ought to be threadsafe but it i don’t recall if its dependencies are (several X libraries, and the X11 Display object).

If xdo needs any changes to enable thread safety, let me know, and hopefully we can make things work together :)

On Sun, May 9, 2021 at 5:47 AM Philipp Keck @.***> wrote:

See also here https://medium.com/dwelo-r-d/wrapping-unsafe-c-libraries-in-rust-d75aeb283c65 :

You can mark your struct Send if the C code dereferencing the pointer never uses thread-local storage or thread-local locking. This happens to be true for many libraries.

You can mark your struct Sync if all C code able to dereference the pointer always dereferences in a thread-safe manner, i.e. consistent with safe Rust. Most libraries that obey this rule will tell you so in the documentation, and they internally guard every library call with a mutex.

---

I think unsafe impl Send for MyStruct {} can be included in this library, right? AFAIU this would allow putting it in a Mutex<>.

I'm not so sure about unsafe impl Sync for MyStruct {}. @jordansissel https://github.com/jordansissel Can you tell whether the xto_t* returned from xdo.h https://github.com/jordansissel/xdotool/blob/master/xdo.h is thread-safe? Given that it's mostly consumed as const (except for the xdo_free function). AFAIU this would allow dropping the Mutex<> again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/crumblingstatue/rust-libxdo/issues/4, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAF2SUCUPNBJ2HEII3A3TTMZ76RANCNFSM44PA7RUA .

[Philipp91]

Here is a good explanation of how thread-safe the underlying Xlib is: If we look at a series of Xlib calls made from libxdo, then each individual function locks on the display, so the Display state should never be badly messed up, but in between the individual calls another thread may interfere, so any other state could be messed up. Broadly, I can see two possible consequences of such interference:

1. External state changes -> different outcome than intended. For instance, imagine a thread executing Ctrl+V with Ctrl down, V down, V up, Ctrl up, and another thread doing the same for Shift+A, then you might inadvertently end up with Ctrl+Shift+V or Ctrl+Shift+A or both. The library shouldn't guard against this. The application may want to guard against this. And if so, it might benefit from having Rust bindings for XLockDisplay and XInitThreads and friends.
2. Internal state invalidated -> Error/crash. For instance, consider the example from the explanation linked above, which ends in "gets BadWindow". Essentially, some previously obtained handle isn't valid anymore. Does libxdo do anything like that? If it keeps no handles around anyway, then libxdo should be thread-safe as is.

[Philipp91]

Some potential issues:

• attr.root -- but then again, if the window whose size you're trying to get gets closed in the meantime, it's fine to receive an error. And _is_success() should handle this correctly. Similarly attr.root here and elsewhere.
• The screen_ret pointer returned here or the window_ret pointer returned here could be outdated by the time the caller gets to use it. Then again it's the caller's problem.
• wattr.screen->root -- then again, if the window to be activated gets closed in the meantime, returning an error is fine.
• root, root, ... -- does the root window ever get invalidated?
• Here if the window gets moved after the in-window coordinates have been determined but before the click is issued, then it will click in the wrong place. I think this is fine to ignore.

Overall, it seems to me that xdolib won't crash in case of concurrency issues, but at worst return something like a BadWindow error in corner-case situations where another thread tampered with the same window at the same time.

There seems to be already some kind of synchronization going on here by "grabbing the cursor".