Infinte Recursion on the position-changed window signal

[kassick]

Describe the bug

The position-changed triggers another position-changed signal. If for some unknown reason the target window resists the computed x,y coordinates, the function causes an infinite recursion.

        this.signals.connect(metaWindow, 'position-changed', w => {
            if (inGrab)
                return;
            let f = w.get_frame_rect();
            let clone = w.clone;
            let x = this.visibleX(w);
            let y = this.monitor.y + clone.targetY;
            x = Math.min(this.width - stack_margin, Math.max(stack_margin - f.width, x));
            x += this.monitor.x;
            if (f.x !== x || f.y !== y) {
                try {
                    w.move_frame(true, x, y);
                }
                catch (ex) {

                }
            }
        });

jan 31 15:43:10 thnkpd gnome-shell[40060]: JS ERROR: too much recursion
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23
                                           addWindow/<@file:///home/kassick/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:857:23

To Reproduce

I don't know what triggered this situation -- I was in slack (the desktop app) and had just clicked a thread to open it in the side view, not as a new window. Maybe some popover hint implemented as a window or something totally unrelated to slack.

System information:
Please execute ./gather-system-info.sh in you PaperWM clone and paste the output below.

Please include this information in your bug report on GitHub!
Distribution: Fedora Linux
GNOME Shell 45.3
Display server: Wayland
PaperWM branch/tag: develop
PaperWM commit: 336ac5058a7f49b36c38023bd03437bb3934eebe
Enabled extensions:
- paperwm@paperwm.github.com
- custom-hot-corners-extended@G-dH.github.com
- dash-to-dock@micxgx.gmail.com
- switcher@landau.fi
- bluetooth-quick-connect@bjarosze.gmail.com
- just-perfection-desktop@just-perfection
- caffeine@patapon.info
- user-theme@gnome-shell-extensions.gcampax.github.com
- appindicatorsupport@rgcjonas.gmail.com

Additional context
Add any other context about the problem here.

[jtaala]

Thanks @kassick,

Tell me - how did you notice this? did this cause any visual discrepancy or bug (which caused you get look at the logs?).

So, this method has been to work around a long-standing issue where the shown window clone's position isn't synced with the actual window.

I usually check with new gnome versions if it's still needed (realised I haven't checked in 45). This just involves commenting out that code block (that adds a position-changed signal to windows) and running it for a while to see if the issue occurs (issue being in tiled windows, the window position is offset / doesn't look right). So far, it doesn't appear in Gnome 45 for me (although I'm just testing now).

For some reason a tiled window's position wasn't synced but it continually didn't move/correct it's frame (hence the recursion).

[jtaala]

Are you also able to comment out that code block (that adds the position-changed handler), logout/login and run for a day or so to see if this worksaround/fix is still needed in Gnome 45?

[kassick]

how did you notice this

The session froze. After I rebooted, I've checked the logs

[kassick]

Are you also able to comment out that code block (that adds the position-changed handler), logout/login and run for a day or so to see if this worksaround/fix is still needed in Gnome 45?

I'll try that!

[jtaala]

Hopefully it's no longer needed. Otherwise, we'll need to put a guard on there to circuit break any recursive calling of the method.

[jtaala]

Update - looks like it's still needed (got an instance of a un-synced clone/window position).

I've created a branch with a recursive guard on this method: #770.

[jtaala]

Closing this as #770 will guard against this.

If this issue pops up again @kassick let me know (we can reopen this then).