Logging to file breaks when using flameshot from multiple processes
Flameshot Version
master https://github.com/flameshot-org/flameshot/commit/1e29613cc0762caa92eed384e25147e5b75f4dc8
Installation Type
Compiled from source
Operating System type and version
linux
Description
The "logging to file" implementation I contributed recently ( #4371 ) does not handle running flameshot from multiple processes. Eg: Running the flameshot daemon but also launch flameshot gui from a terminal to take a one off image.
There is a static mutex that protects against multiple threads from within the same process from writing/rotating the log file concurrently, but this does not protect multiple processes. In the best case this means the log file gets overwritten and doesn't contain any useful info. In the worst case flameshot might crash or behave poorly if a file on disk is deleted while it still has the QFile handle open. No crash happens on my system, but I'm not able to test on windows/macosx.
I've tried adding a shared memory flag to coordinate multiple processes rolling/accessing the log file (inspired by the guiMutex) , but I'm not super familiar with the Qt6 IPC/SharedMemory APIs and I'm not sure if I'm doing it right, it also seems fragile on Unix when considering crashed processes.
Another possible solution would be to change the "rolling" algorithm to simply check file size and roll if the file is over the max size, keeping on old file to ensure continuity. This approach would not stop multiple flameshot processes from writing over eachother though.
I think until this issue gets fixed it might be a good idea to disable logging to file by default.
I will keep working on implementing a fix but don't have a ton of time over the next two weeks. If anyone has any guidance on the best way to handle this issue, I would appreciate it! I'm not very familiar with Qt6.
Steps to reproduce
- run flameshot in daemon mode (ie with system tray icon)
- Take a screenshot using the system tray icon and copy to clipboard (so a message is logged)
- run flameshot via
flameshot guiin a terminal and copy a screenshot to clipboard (so a message is logged) - Observe contents of log file (`$HOME/.local/state/flameshot/flameshot.log) - it will only contain the log message(s) procuded by the second screenshot.
Screenshots or screen recordings
No response
System Information
linux debian testing - wayland
Good catch. Two quick ideas
-
Append the PID to the log file name, but that could make rotation difficult
-
Use KSingleApplication to send messages from the cli invocation to the daemon if it detects its running as the second instance. Only primarily instances are allowed to write the files.
I'm going to revert #4371 and we can just resubmit that PR when fixed.