Skip to content

screenshot-manager: don't wedge on a failed screencopy - #7

Open
wayne-tbl wants to merge 1 commit into
FuriLabs:forkyfrom
wayne-tbl:screenshot-failure-wedge
Open

screenshot-manager: don't wedge on a failed screencopy#7
wayne-tbl wants to merge 1 commit into
FuriLabs:forkyfrom
wayne-tbl:screenshot-failure-wedge

Conversation

@wayne-tbl

@wayne-tbl wayne-tbl commented Aug 19, 2026

Copy link
Copy Markdown

One failed screenshot currently disables screenshots until phosh is restarted.

maybe_screencopy_done() completes the D-Bus invocation itself on the failure
path and returns without disposing self->frames:

  /* With a failure no need to merge pixbufs */
  if (failed) {
    phosh_dbus_screenshot_complete_screenshot (PHOSH_DBUS_SCREENSHOT (self),
                                               self->frames->invocation,
                                               FALSE,
                                               self->frames->filename ?: "");
    return;
  }

Two problems.

The invocation may be NULL. An internally requested shot — the power
button, or anything else calling phosh_screenshot_manager_take_screenshot()
— has none, so the failure path asserts:

g_dbus_method_invocation_return_value_internal:
assertion 'G_IS_DBUS_METHOD_INVOCATION (invocation)' failed

self->frames is never disposed, on any failure path, D-Bus included.
phosh_screenshot_manager_do_screenshot() refuses to start while it is set:

  if (self->frames) {
    g_debug ("Screenshot already in progress");
    return FALSE;
  }

so every screenshot after the first failure is rejected before it reaches
screencopy.

screenshot_done(), a few lines up in the same file, already handles both
cases — it completes the invocation only when there is one, and disposes the
frames either way. This routes the failure through it.

Reproduced

On a FuriPhone FLX1, by taking a screenshot with the screen blanked, where wlr
screencopy cannot read the hwcomposer output:

phosh: Failed to copy output 'HWCOMPOSER-1'
phosh: g_dbus_method_invocation_return_value_internal:
       assertion 'G_IS_DBUS_METHOD_INVOCATION (invocation)' failed

The next org.gnome.Shell.Screenshot.Screenshot call then returned
(false, '') immediately, with no Failed to copy output line of its own
i.e. it never reached screencopy.

That it is the stuck frames rather than the blanked screen was confirmed by
waking the display and trying again: still (false, ''), still instant, still
no log line. It stayed that way until phosh was restarted, after which the
identical call returned (true, '/home/furios/Pictures/…png') and wrote a
valid 1080x2416 PNG.

With the patch the failure disposes the frames, so the following screenshot
proceeds normally instead of inheriting the wedge.

Notes

  • One hunk, no behaviour change on the success path.
  • Independent of the other open PRs; based on current forky.

maybe_screencopy_done() completed the D-Bus invocation itself on the
failure path and returned without disposing self->frames. Two things
follow from that.

An internally requested screenshot -- the power button, or anything else
reaching phosh_screenshot_manager_take_screenshot() with a NULL
invocation -- has nothing to complete, so it tripped

  g_dbus_method_invocation_return_value_internal:
  assertion 'G_IS_DBUS_METHOD_INVOCATION (invocation)' failed

And since self->frames stayed set -- on every failure path, not just the
internal one -- phosh_screenshot_manager_do_screenshot() refused every
later request with "Screenshot already in progress". So one failed
screenshot disabled screenshots until phosh was restarted.

screenshot_done() already handles both cases: it completes the
invocation only when there is one, and disposes the frames either way.
Route the failure through it.

Reproduced on a FuriPhone FLX1 by shooting with the screen blanked,
where wlr screencopy cannot read the output. The first request logged
"Failed to copy output 'HWCOMPOSER-1'" and the assertion above; every
request after it returned (false, '') immediately, without reaching
screencopy at all.
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.

1 participant