search

woodruffw / x_do.cr

Crystal bindings for libxdo (xdotool)
https://woodruffw.github.io/x_do.cr/
MIT License
18 stars 1 forks source link

Invalid memory access when searching for windows #10

Open phil294 opened 2 years ago

phil294 commented 2 years ago

Hi, thanks for writing these bindings, they are very useful and easy to work with. However, window searching doesn't work:

require "x_do"

XDo.act do
    winds = search { window_name "Audacious" }
    puts winds
end

results in

Invalid memory access (signal 11) at address 0x7ffe00000000
[0x55ee897bffc6] *Exception::CallStack::print_backtrace:Nil +118 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x55ee897af9b6] ~procProc(Int32, Pointer(LibC::SiginfoT), Pointer(Void), Nil) +310 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x7f6bf9cdd8e0] ?? +140101729245408 in /usr/lib/libc.so.6
[0x7f6bf9dff7fd] ?? +140101730433021 in /usr/lib/libc.so.6
[0x7f6bf9d93bbc] regcomp +156 in /usr/lib/libc.so.6
[0x7f6bfa0f9e3a] ?? +140101733555770 in /usr/lib/libxdo.so.3
[0x7f6bfa0f9fa0] ?? +140101733556128 in /usr/lib/libxdo.so.3
[0x7f6bfa0fa9a3] xdo_search_windows +163 in /usr/lib/libxdo.so.3
[0x55ee89831e36] *XDo#search<XDo::Search>:Array(XDo::Window) +422 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x55ee8979faf7] __crystal_main +1191 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x55ee89833fa6] *Crystal::main_user_code<Int32, Pointer(Pointer(UInt8))>:Nil +6 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x55ee89833f09] *Crystal::main<Int32, Pointer(Pointer(UInt8))>:Int32 +41 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x55ee897ace16] main +6 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x7f6bf9cc8290] ?? +140101729157776 in /usr/lib/libc.so.6
[0x7f6bf9cc834a] __libc_start_main +138 in /usr/lib/libc.so.6
[0x55ee8979f575] _start +37 in /home/phi/.cache/crystal/crystal-run-testi.tmp
[0x0] ???

active_window and win.type work without issue however.

Is this a Crystal version thing? Or just my machine? I don't really know how to proceed now, my project kind of depends on it :|

woodruffw commented 2 years ago

Thanks for the report! It could be any of those things. Could you provide the following?

My random guess is that libxdo changed underneath this library (I don't update it very often), so it might just need a small fix.

phil294 commented 2 years ago

-

   crystal --version
   Crystal 1.4.1 (2022-04-23)

   LLVM: 13.0.1
   Default target: x86_64-pc-linux-gnu

I have another suspicion that it might be some kind of permission error. Reason: yesterday, I tried out some simple native X11 code in C++ to listen for key and focus events, and it worked fine. When I ported it identically to Crystal using x11-cr, the code also worked, but on most windows it failed with X Error of failed request: BadAccess (attempt to access private resource denied) kind of stuff. And with x_do, I also noticed that win.type also fails in several windows (without message, nothing happens), while with normal xdotool getactivewindow type asdf, everything works as expected. So perhaps also the error this very issue is about is due to some basic X11 Crystal permission problem. I'll investigate this further tomorrow. TL;DR: I have no idea

woodruffw commented 2 years ago

Thanks for the information! Yeah, a permission issue is possible. It might be fruitful to try to write a small C program that calls the same libxdo APIs as x_do.cr and see if it also crashes (or produces an obvious error).

I'll also try to set aside some time tomorrow to reproduce the bug using your sample above.

phil294 commented 2 years ago

So I checked with the previous release of libxdo (which dates to solid 6 years before the recent one), turns out it works! So yeah, apparently some breaking changes from the underlying library which is worrying :S

phil294 commented 2 years ago

that said, even with the 2016 version, most (all?) window actions such as minimize! don't work for me, and sending keys fails in some windows, while xdotool works in all cases.

woodruffw commented 2 years ago

Aha! Okay, it's likely that some of the C APIs changed underneath the binding. They're not automatically synchronized, so that's not surprising. I can take a look tonight or tomorrow.

phil294 commented 2 years ago

Found the culprit, it's the introduction of search param windowrole. I have a local branch where I added this param and everything including tests runs fine. I wrote to jsissel here about breaking changes: https://github.com/jordansissel/xdotool/pull/305#issuecomment-1195016978

Now what do we do? Replace struct Search with two alternatives struct Search2016 and struct Search2021 and somehow figure out what the user has installed? Ship libxdo with the source somehow? Drop 2016 support? The 2016 version is still the default in all existing Ubuntu versions so that would mean dropping support for the majority of users.

I have no experience with C or binding conventions or Linux build systems worth mentioning, so I have come to a halt now. The gruesome nature of C development is what led me to Crystal in the first place, so I thought I'd never have to deal with these problems, yet here we are...

phil294 commented 2 years ago

Further API changes (new features, git diff 8a6aecee9..HEAD -- xdo.h) since the 2016 version are the addition of xdo_get_window_classname and xdo_window_state and xdo_quit_window. I'll also send PR for these some day, since I need them as well

woodruffw commented 2 years ago

Thanks a ton for the root cause effort!

The 2016 version is still the default in all existing Ubuntu versions so that would mean dropping support for the majority of users.

Gah. That's not ideal...

IMO, we should attempt to detect the version the user has installed (no idea how to do that, yet), and attempt to macro in a single struct Search based on that. That way we only have one copy of the struct in the compiled result, corresponding to the user's needs.

I'll have more free time to look into this during the coming weekend; I know enough about the C and toolchains side to probably hack a solution together :slightly_smiling_face:

phil294 commented 2 years ago

relevant issue on libxdo breaking changes

Edit: I haven't heard anything from the xdotool author about this and am worried this kind of stuff will repeat. So I'm now simply linking libxdo statically. shards build -Dpreview_mt --link-flags="$PWD/static/libxdo.a -lxkbcommon -lXinerama -lXtst -lXi -lX11" (the last -l ones are libxdo dependencies)

Edit2: I have also implemented window_state and quit_window locally now (2 of the other 3 2021x features). Once this issue here is resolved, I will send PRs to target those in the newer version.

Edit3: Same with set_active_modifiers and clear_active_modifiers and get_class_name. https://github.com/phil294/x_do.cr/commits/add-window-get-class-name

woodruffw commented 1 year ago

@phil294 is this resolved by https://github.com/jordansissel/xdotool/pull/434, or is that a slightly different bug? 🙂

phil294 commented 1 year ago

definitely something else. The linked PR fixes some very exotic error around invisible windows, but this very issue is about the incompatibility with xdotool version 2021 and beyond in general. I think the only breaking change is in the search struct.