common/commands: Do not use await to avoid race condition

[easyteacher]

Analysis from nyanpasu64:

On KDE Plasma X11, with Klipper (clipboard history) added to plasmashell,
and Firefox with "Copy Selected Tabs to Clipboard" installed, trying
to use the extension often causes Firefox and plasmashell (and any app
trying to paste) to deadlock and hang.

When you use the extension to copy a tab list, Firefox sends a "clipboard
changed" event (through X11 I assume, not sure) and attempts to send a
notification through D-Bus (notify_notification_show () at /usr/lib/libnotify.so.4 blocks on g_dbus_connection_send_message_with_reply_sync).

Meanwhile plasmashell can't process the notification and unblock Firefox,
because the Klipper plasmashell plugin is blocked trying to grab the
clipboard contents (QMimeData::text() blocks on QXcbClipboard::getSelection
and QXcbClipboard::waitForClipboardEvent). plasmashell/Klipper asks Xorg to ask
Firefox for clipboard contents. But Firefox can't provide clipboard contents
until it's done showing a notification, and plasmashell won't show the
notification until it gets the clipboard contents.

And both apps lock up for 10 seconds. And in the end, plasmashell gives up trying
to grab clipboard contents, processes the notification, and both programs get
unblocked (but Klipper is missing the clipboard entry).

To avoid hanging, we just need to remove await so the race condition can
be effectively avoided.

See also: https://bugs.kde.org/show_bug.cgi?id=446581

[nyanpasu64]

Tried installing in Firefox Developer 95.0b12 (from Arch Linux), with xpinstall.signatures.required=false. This PR does not fix the race condition hang. Firefox and plasmashell still hang for 11(!) seconds when I try copying tab URLs. I don't think that removing a call to await can prevent the deadlock I described. Firefox still blocks on sending a notification, and plasmashell still blocks on reading clipboard data.

Additionally not calling await makes me uneasy, since it's effectively spawning a background task which you'll never know if it succeeds or fails. In this particular case it's not as big of a deal, but since this PR doesn't fix the hang, I wouldn't do it.

[piroor]

@easyteacher Changes on this PR will loose error handling, so I've introduced alternative changes with the commit 6b37e0d. It is reasonable to make more safe for methods resolved with delay, even if it does not fix the original problem.

[easyteacher]

@easyteacher Changes on this PR will loose error handling, so I've introduced alternative changes with the commit 6b37e0d. It is reasonable to make more safe for methods resolved with delay, even if it does not fix the original problem.

Oh that's my mistake. I have also tried using then but it will make Firefox definitely hang after copying.

[piroor]

@nyanpasu64 Thank you for trying. Is it originally a bug of Firefox itself or a combination issue of Firefox and the plasmashell?

[piroor]

I have also tried using then but it will make Firefox definitely hang after copying.

Hmm... I can't imagine how to solve this problem on my addon.

[nyanpasu64]

For what it's worth, trunk a3fb3bf (including 6b37e0d) still hangs. I'm not sure if it hangs less often or not.

@nyanpasu64 Thank you for trying. Is it originally a bug of Firefox itself or a combination issue of Firefox and the plasmashell?

It's a deadlock, meaning it arises from the interaction between two apps, and is a design flaw and not necessarily a flaw of either app in isolation. Given my limited expertise, I think that it's reasonable for Firefox to copy something to the clipboard and then show a notification and expect it to finish synchronously, and it's unreasonable for plasmashell to assume that apps will respond to clipboard content requests, and to stop processing notifications while waiting for clipboard contents. So perhaps plasmashell and Klipper should be rewritten to not block while waiting for clipboard contents, but that's quite involved (perhaps asynchronously, which may require bypassing QXcbClipboard entirely, perhaps having background threads request clipboard contents).

Alternatively Firefox could use an asynchronous function instead of libnotify's notify_notification_show, to respond to clipboard requests while trying to show a notification. I'm not sure how difficult that is, AKA how many round-trips d-bus's notification protocol requires, and how many states need to be created in Firefox's state machine.

[piroor]

Oh I completely misunderstood what is the problem. Currently this addon does not have option to disable the notification after copying. Introducing such an option may become a workaround, how about the idea?

[piroor]

Introducing such an option may become a workaround, how about the idea?

Oops it already exists: configs.shouldNotifyResult. It corresponding to the checkbox "Show a desktop notification notification when successfully copied (or failed)" in the options page of this addon.

[easyteacher]

Close it as the bug does not belong to here.