Skip to content

subprojects: wlroots: Update to 0.20.0#3002

Open
soreau wants to merge 2 commits intomasterfrom
update-wlroots-0.20.x
Open

subprojects: wlroots: Update to 0.20.0#3002
soreau wants to merge 2 commits intomasterfrom
update-wlroots-0.20.x

Conversation

@soreau
Copy link
Copy Markdown
Member

@soreau soreau commented Mar 27, 2026

No description provided.

@soreau soreau force-pushed the update-wlroots-0.20.x branch from 751f704 to 6515c18 Compare March 27, 2026 16:36
@soreau soreau force-pushed the update-wlroots-0.20.x branch from 6515c18 to 8d45964 Compare March 27, 2026 16:39
@mark-herbert42
Copy link
Copy Markdown
Contributor

It will most likely cause crashes here

    auto evt_input_method = static_cast<wlr_input_method_v2*>(data);
    assert(evt_input_method == input_method);

Try to open wcm and change some settings here, open-close windiows etc. And it will crash, as data is no longer passed to wayland by wroots, data pointer is simply dropped and wlroots pass NULL to wayland instead of this.

I do revert those changes in local copy of wlroots (in multiple locations) - so it s stable now , but with vanilla wlroots it crashes. Not immediately - you need to do something, so it can run for a while and it compiles like a charm. But it is a stinker bomb under wayfire. I have no idea why this shitty commit landed in wlroots and how to fix it ob wayfire without reverting wlroots, but maybe you or ammen99 have better idea. Bad thing - it is not always crashes. so tests can pass

  •   wl_signal_emit_mutable(&text_input->events.destroy, NULL);
    
  •   wl_signal_emit_mutable(&text_input->events.destroy, text_input);
    

@mark-herbert42
Copy link
Copy Markdown
Contributor

wlr_input_method_manager_v2.events.commit & wlr_input_method_manager_v2.events.destroy signals now use NULL sources as data instead of the wlr_input_method_v2 owner.

That's the change causing the trouble.

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

wlr_input_method_manager_v2.events.commit & wlr_input_method_manager_v2.events.destroy signals now use NULL sources as data instead of the wlr_input_method_v2 owner.

That's the change causing the trouble.

Can you possibly send a PR targeting this branch?

@mark-herbert42
Copy link
Copy Markdown
Contributor

Here is the troublemaking merge.
https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5107

@akliuxingyuan
Copy link
Copy Markdown
Contributor

akliuxingyuan commented Mar 28, 2026

#3003 could you please try this patch

@mark-herbert42
Copy link
Copy Markdown
Contributor

does not help much.
wayfire.log

@mark-herbert42
Copy link
Copy Markdown
Contributor

mark-herbert42 commented Mar 28, 2026

wayfire: ../wayfire-0.10.1.1/src/core/seat/input-method-relay.cpp:426: wf::text_input::text_input(wf::input_method_relay*, wlr_text_input_v3*)::<lambda(void*)>: Assertion `input == wlr_text_input' failed.
(EE) failed to read Wayland events: Broken pipe

One more place where this one is called, and what is worse - after assertion this pointer is used. So just removing the assertion will not help from crash.

To reproduce crash - run wcm, try to do some change in text field, then close wcm. Crash will follow.

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

@mark-herbert42 The first thing that jumps out at me in your log is the first line.

II 2026-03-28 11:58:01.929 - [wayfire-0.10.1.1/src/main.cpp:410] Starting wayfire: 0.11.0-unknown (Mar 28 2026) branch unknown wlroots-0.20.0-rc5

Apparently you are not even using the branch in this pull request, nor the released version in the wlroots subproject. Please do not report issues unless you are actually using the wayfire update-wlroots-0.20.x branch with the wlroots-0.20.0 release.

@mark-herbert42
Copy link
Copy Markdown
Contributor

wayfire.log
I was used the wlroots fork for vulkan effects, now rebuilt everythong with pure vanilla 0.20 release tarball, and wayfire source using master branch + your PR with latest updates. Does not change much - in fact does not change anything. And it can not and should not - the breaking commit is in wlroots 0.20 branch even before rc1 and nothing changed since then.
Open WCM, close WCM - enjoy the crash. Well I know how to patch wlroots to make the thing work again -
revert_breaking_commit.patch

if you apply this over your wlroots version it will work, but how I understand the idea Wayfire need to work with wlroots release how it is done by wlroots team, not using some special wlroots version

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

@mark-herbert42 Thank you for testing this. I was unable to produce the crash and I am unsure about the exact mechanics of this plugin, but does this help the crash while preserving functionality?

diff --git a/src/core/seat/input-method-relay.cpp b/src/core/seat/input-method-relay.cpp
index ca26291f..0a4a0037 100644
--- a/src/core/seat/input-method-relay.cpp
+++ b/src/core/seat/input-method-relay.cpp
@@ -372,9 +372,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 {
     on_text_input_enable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (relay->input_method == nullptr)
         {
             LOGI("Enabling text input, but input method is gone");
@@ -388,9 +385,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_commit.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (!input->current_enabled)
         {
             LOGI("Inactive text input tried to commit");
@@ -414,30 +408,16 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_disable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         relay->disable_text_input(input);
     });
 
     on_text_input_destroy.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
-        if (input->current_enabled)
-        {
-            relay->disable_text_input(wlr_text_input);
-        }
-
         set_pending_focused_surface(nullptr);
         on_text_input_enable.disconnect();
         on_text_input_commit.disconnect();
         on_text_input_disable.disconnect();
         on_text_input_destroy.disconnect();
-
-        // NOTE: the call destroys `this`
-        relay->remove_text_input(wlr_text_input);
     });
 
     on_pending_focused_surface_destroy.set_callback([&] (void *data)

@mark-herbert42
Copy link
Copy Markdown
Contributor

mark-herbert42 commented Mar 28, 2026

wayfire.log

The patch is exactly what I was trying myself - if assert fails, remove assert. So i did - but seems that assert was doing something on purpose, and simply patching that out caused segfaults and a lot of trouble. But the issue is - if you do not have crash, maybe I have something enabled that you dont.

Seems you might have input-method-v2 disabled in workarounds settings. When I uncheck it and restart wayfire - no crashes, and that's also understandable as the bad commint is in input v2 code.

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

@mark-herbert42 Thanks, I was able to reproduce the crash indeed by enabling the input-method-v2 workaround option. This seems to help here, are you able to confirm it doesn't crash and does whatever it is expected to do?

index ca26291f..5e25b867 100644
--- a/src/core/seat/input-method-relay.cpp
+++ b/src/core/seat/input-method-relay.cpp
@@ -329,6 +329,10 @@ void wf::input_method_relay::set_focus(wlr_surface *surface)
 {
     for (auto & text_input : text_inputs)
     {
+        if (!text_input->input->current_enabled)
+        {
+            continue;
+        }
         if (text_input->pending_focused_surface != nullptr)
         {
             assert(text_input->input->focused_surface == nullptr);
@@ -372,9 +376,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 {
     on_text_input_enable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (relay->input_method == nullptr)
         {
             LOGI("Enabling text input, but input method is gone");
@@ -388,9 +389,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_commit.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (!input->current_enabled)
         {
             LOGI("Inactive text input tried to commit");
@@ -414,30 +412,16 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_disable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         relay->disable_text_input(input);
     });
 
     on_text_input_destroy.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
-        if (input->current_enabled)
-        {
-            relay->disable_text_input(wlr_text_input);
-        }
-
         set_pending_focused_surface(nullptr);
         on_text_input_enable.disconnect();
         on_text_input_commit.disconnect();
         on_text_input_disable.disconnect();
         on_text_input_destroy.disconnect();
-
-        // NOTE: the call destroys `this`
-        relay->remove_text_input(wlr_text_input);
     });
 
     on_pending_focused_surface_destroy.set_callback([&] (void *data)

@mark-herbert42
Copy link
Copy Markdown
Contributor

unfortunately not - same bad crash
wayfire.log

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

@mark-herbert42 I see you have enabled asan, but somehow it is not printing line numbers. Have you passed -Dsanitize=address,undefined to meson? or are you using something else? It would be nice if you could present a trace that referenced the filenames and line numbers.

@mark-herbert42
Copy link
Copy Markdown
Contributor

fix2.patch
Seems I managed to make it not dump and it looks OK - but I do not know what it does and if it is leaking something or doing something other badly.

I've not used Dsanitize - I am actualy emerging gento ebuild, just feeding some patches to it. Will try to change ebuild and revert my fix.

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

How are you applying the patch(es) I am sending? Because it seems that you do not need to do this if you applied the suggested patch correctly.

@mark-herbert42
Copy link
Copy Markdown
Contributor

adding the to ebuild
eapply "${FILESDIR}/fix1.patch"
and putting the patch contents you send in the file.
wayfire.log

            -Dbuildtype=debug
            -Db_sanitize=address,undefined
            -Dxwayland=auto

here are my meson options for wayfire. But wlroots build separately.

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

            -Dbuildtype=debug
            -Db_sanitize=address,undefined
            -Dxwayland=auto

here are my meson options for wayfire. But wlroots build separately.

I'm guessing it builds with debug symbols but then strips the binary. Can you tell it to not strip?

@mark-herbert42
Copy link
Copy Markdown
Contributor

So far my little workaround without the last patch applied it works so far.

replacng this
auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
assert(input == wlr_text_input);

with this
auto wlr_text_input = input;

allows all the further code to run correctly. My guess is - before wlroots was sending the pointer to wayland, so in this pointer we were receiving the "result" of wayland call. So the assert was actually comparing the cached input with the data pointer contents to make sure it allright. But now wlroots is doing call blindly returning this NULL so assert between NULL and cached input is failing and it dumps before actual dump. So I assume that wlroots call always good so simply passing the input instead of data into the further code, hoping that it is allright. It works so far - anyway in old code if data not equal to input the assert will fail and crash. So now it also will crash if the wlroots call failed.... But that's a guess - I am not an expert in this and it iw beyound my knowledge/

here is the log with your last patch applied and nostrip. indeed it was stripping that removed the line numbers etc, so your guess is 100% right - here are the code details. It is just your last patch with asserts and some other operations removed completely.

wayfire.log

@soreau
Copy link
Copy Markdown
Member Author

soreau commented Mar 28, 2026

@mark-herbert42 The whole point of having the line numbers with a specific patch is so that I can have eyes on the code where it's failing. If you change other things, the line numbers might change too, and then I am left in the dark. Please apply this patch with no other patches or changes, and let me know if it works for you. We need to try and get this pushed to the branch so that it works for everyone, which is the ultimate goal here.

diff --git a/src/core/seat/input-method-relay.cpp b/src/core/seat/input-method-relay.cpp
index ca26291f..bee854f0 100644
--- a/src/core/seat/input-method-relay.cpp
+++ b/src/core/seat/input-method-relay.cpp
@@ -329,6 +329,11 @@ void wf::input_method_relay::set_focus(wlr_surface *surface)
 {
     for (auto & text_input : text_inputs)
     {
+        if (!text_input || !text_input->input ||
+            !text_input->input->current_enabled)
+        {
+            continue;
+        }
         if (text_input->pending_focused_surface != nullptr)
         {
             assert(text_input->input->focused_surface == nullptr);
@@ -372,9 +377,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 {
     on_text_input_enable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (relay->input_method == nullptr)
         {
             LOGI("Enabling text input, but input method is gone");
@@ -388,9 +390,6 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_commit.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (!input->current_enabled)
         {
             LOGI("Inactive text input tried to commit");
@@ -414,20 +413,14 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
 
     on_text_input_disable.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         relay->disable_text_input(input);
     });
 
     on_text_input_destroy.set_callback([&] (void *data)
     {
-        auto wlr_text_input = static_cast<wlr_text_input_v3*>(data);
-        assert(input == wlr_text_input);
-
         if (input->current_enabled)
         {
-            relay->disable_text_input(wlr_text_input);
+            relay->disable_text_input(input);
         }
 
         set_pending_focused_surface(nullptr);
@@ -437,7 +430,7 @@ wf::text_input::text_input(wf::input_method_relay *rel, wlr_text_input_v3 *in) :
         on_text_input_destroy.disconnect();
 
         // NOTE: the call destroys `this`
-        relay->remove_text_input(wlr_text_input);
+        relay->remove_text_input(input);
     });
 
     on_pending_focused_surface_destroy.set_callback([&] (void *data)

@mark-herbert42
Copy link
Copy Markdown
Contributor

okay - so applied this PR over clean wayfire git master + the last patch. And this one works like a charn - no crashes. tried all the tricks that cases crashes before and still up and running. WCM, Telegram, firefox - all working well. Hope to see this patch as part of this PR to merge it all cleanly over master branch , and better to see the whole PR merged there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants