Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when daemon is not started and disabledTrayIcon=true #1730

Closed
chuahou opened this issue Jul 21, 2021 · 10 comments · Fixed by #1742
Closed

Crash when daemon is not started and disabledTrayIcon=true #1730

chuahou opened this issue Jul 21, 2021 · 10 comments · Fixed by #1742
Labels
Unconfirmed Bug The bug is not confirmed by anyone else.

Comments

@chuahou
Copy link
Contributor

chuahou commented Jul 21, 2021

Flameshot version

$ flameshot -v
Flameshot v0.9.0
Compiled with Qt 5.15.2

Describe the bug

When

  • the flameshot daemon is not running (ps aux | grep flameshot), such as after killall flameshot, and
  • disabledTrayIcon=true

running flameshot crashes. flameshot config and flameshot gui similarly crash but without anything printed to the terminal.

$ flameshot
QSettings::value: Empty key passed
QSettings::value: Empty key passed
QSettings::setValue: Empty key passed
QSettings::value: Empty key passed
QSettings::setValue: Empty key passed
Segmentation fault (core dumped)

If the flameshot daemon is running (e.g. by running a flameshot command with disabledTrayIcon=false, and disabledTrayIcon is later set to true, new invocations of flameshot gui and flameshot config no longer crash.

To Reproduce

  1. Kill the flameshot daemon (killall flameshot).
  2. Set disabledTrayIcon=true in ~/.config/flameshot/flameshot.ini.
  3. Run any flameshot command.

The configuration used to produce this bug (generated by first run of flameshot, with disabledTrayIcon modified, is shown here.

Configuration flameshot.ini
[General]
contrastOpacity=188
disabledTrayIcon=true
drawColor=#ff0000
drawThickness=0
ignoreUpdateToVersion=0.10.0
savePath=/home/sgepk/media
savePathFixed=false
showStartupLaunchMessage=true
startupLaunch=true

[Shortcuts]
TYPE_ARROW=A
TYPE_CIRCLE=C
TYPE_CIRCLECOUNT=
TYPE_COMMIT_CURRENT_TOOL=Ctrl+Return
TYPE_COPY=Ctrl+C
TYPE_DRAWER=D
TYPE_EXIT=Ctrl+Q
TYPE_IMAGEUPLOADER=Return
TYPE_MARKER=M
TYPE_MOVESELECTION=Ctrl+M
TYPE_MOVE_DOWN=Down
TYPE_MOVE_LEFT=Left
TYPE_MOVE_RIGHT=Right
TYPE_MOVE_UP=Up
TYPE_OPEN_APP=Ctrl+O
TYPE_PENCIL=P
TYPE_PIN=
TYPE_PIXELATE=B
TYPE_RECTANGLE=R
TYPE_REDO=Ctrl+Shift+Z
TYPE_RESIZE_DOWN=Shift+Down
TYPE_RESIZE_LEFT=Shift+Left
TYPE_RESIZE_RIGHT=Shift+Right
TYPE_RESIZE_UP=Shift+Up
TYPE_SAVE=Ctrl+S
TYPE_SELECTION=S
TYPE_SELECTIONINDICATOR=
TYPE_SELECT_ALL=Ctrl+A
TYPE_TEXT=T
TYPE_TOGGLE_PANEL=Space
TYPE_UNDO=Ctrl+Z

Expected behavior

The daemon starts without crashing.

System Information

NixOS 21.05 i3

Regarding FAQs: A system tray and notification daemon are both available, and have been used by flameshot with no problem prior to this. In fact, flameshot still works now when the steps above are not taken.

@mmahmoudian mmahmoudian added the Unconfirmed Bug The bug is not confirmed by anyone else. label Jul 21, 2021
@mmahmoudian mmahmoudian self-assigned this Jul 21, 2021
@mmahmoudian
Copy link
Member

mmahmoudian commented Jul 21, 2021

On Manjaro KDE Xorg with flameshot v0.10.0, I get the same output as you when running flameshot, but it does not crash, it appears in the processes and the flameshot gui works as expected.

Unfortunately at the time of writing this comment, the version of Flameshot in NixOS repository is still 0.9.0 and this might be a bug in that version.

@mmahmoudian mmahmoudian removed their assignment Jul 21, 2021
@chuahou
Copy link
Contributor Author

chuahou commented Jul 21, 2021

Thanks for even checking the packaging status for my distro, @mmahmoudian! I'll try to get 0.10.0 installed and see if I can still replicate this crash.

@chuahou
Copy link
Contributor Author

chuahou commented Jul 21, 2021

The crash does not occur on 0.10.0 on my system, so it has probably been fixed somewhere between 0.9.0 and 0.10.0.

Funnily enough, the first launch edited my flameshot.ini and reset the value to false, but changing it back to true and relaunching worked perfectly fine.

Closing this issue since it's not present in the latest release. Is there an intention to backport bugfixes to 0.9.0? If so, please let me know and I'll reopen the issue to track a bug present in the older version.

@chuahou chuahou closed this as completed Jul 21, 2021
@mmahmoudian
Copy link
Member

Thanks for trying it out and reporting back. It always feels good the the person who opens an issue shows interest in investigating and solving the problem 🍻

I don't see why backporting should be done as all distros should gradually catch up with the latest version. I don't know how frequently NixOS updates their packages repo, but to my understanding they are not like Debian or Ubuntu that takes years. Also imho backporting is useful when a necessary feature is removed from a software (e.g hardware support), but in case of Flameshot, we have not removed any functionality or OS support, even the QT version is the same. Therefore backporting will just add extra workload to the devs. That said, I'm not a developer in this repo and I just do triage and maintenance of website and documentation. Perhaps @borgmanJeremy can answer you better in this matter.

@chuahou
Copy link
Contributor Author

chuahou commented Jul 21, 2021

That makes sense. The update to 0.10.0 is making its way through nixpkgs already, so that's not my concern. I'm not familiar with flameshot's versioning policy which is why I thought I'd ask just in case, but in hindsight I realized backporting really doesn't make that much sense (I forgot I'm looking at an executable rather than a library).

Thanks for your help with this issue!

@hosiet
Copy link
Member

hosiet commented Jul 22, 2021

My concern is that there could be something buggy within the source code that checks update. If that is the case, the current v0.10.0 would also crash whenever a new version (e.g., v0.11.0) is available on internet. Upgrading only hides the problem, not solves the problem.

Of course, it would be best if the issue has already been fixed after v0.9.0. We could wait and see what happens with the next release.

@chuahou
Copy link
Contributor Author

chuahou commented Jul 22, 2021

@hosiet I think this bug is (most likely) related to tray icon configuration rather than checking for updates, could you be thinking of another issue?

@hosiet
Copy link
Member

hosiet commented Jul 22, 2021

@chuahou I believe I am referencing the correct issue. See https://bugs.debian.org/991320 for detailed analysis on the bug. I will test it first and propose a patch if everything is OK.

hosiet pushed a commit to hosiet/flameshot that referenced this issue Jul 22, 2021
Fix crash when the user has disabledTrayIcon=true, but also
with checkForUpdates=true (either explicitly or implicitly).

closes: flameshot-org#1721 .
closes: flameshot-org#1730 .
See also: https://bugs.debian.org/991320

Signed-Off-By: Boyuan Yang <byang@debian.org>
@chuahou
Copy link
Contributor Author

chuahou commented Jul 23, 2021

@hosiet I see, very nice! I wasn't aware of the discussion on the Debian bugtracker.

@borgmanJeremy
Copy link
Contributor

I'll release a 10.1 fix this weekend. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Unconfirmed Bug The bug is not confirmed by anyone else.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants