Page MenuHome GnuPG

Kleopatra / Gpgtar: Cancel on encrypt leaves a broken archive behind
Testing, NormalPublic

Description

Follow up for T6524 and T6530:

Cancelling a running gpgtar encrypt process via the "Abort" button in Kleopatra leaves an unfinished tar.gpg file behind.
There is no information shown to the user, the background processes are silently stopped.

Expected behavior:
Unfinished (and thereby broken) archives are removed.
If that is not possible, at least a warning should be shown that the process has generated a broken archive and one should delete it manually.

Details

Version
Gpg4win-4.2.0-beta360+

Event Timeline

Note that Kleopatra has code that should take care of removing a left-over file. See also T6530#172608.

aheinecke moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
aheinecke added a subscriber: aheinecke.

Eva this was still in the backlog. But I think it is fixed. Can you check please?

It is present in gpg4win 4.2.0. I do not have a later testversion.

aheinecke moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

Oh, then its back to the backlog

Kleopatra should (try to) delete the file: rKLEOPATRA59652a394662: Remove the output file if job was canceled or an error occurred. The attempt to delete the file is logged, so you should see the logs in the debug output. If nothing is logged, then something weird is happening. Or the QFile::exists() check does not work for some reason.

Debugview shows:

[8412] org.kde.pim.kleopatra: Collection Progress:  48  total:  1000
[8412] org.kde.pim.kleopatra: 
[8412] org.kde.pim.kleopatra: 0x9cfca58 slotWizardCanceled
[8412] org.kde.pim.kleopatra: Kleo::Crypto::SignEncryptFilesController(0x64c97a8) cancel
[8412] org.kde.pim.kleopatra: Kleo::Crypto::SignEncryptTask(0xeee73c0) cancel
[8412] org.kde.pim.kleopatra: Error:  "Abbruch durch Benutzer"
[8412] org.kde.pim.kleopatra:

I think the problem is that SignEncryptTask is destroyed before the canceled job reports the result. Therefore the clean-up code never runs. The added logging should confirm this.

ikloecker moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
ikloecker moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

The sign/encrypt archive jobs now also take care of removing the output file if the operation was canceled or failed.

I have no idea whether this fixes the missing cleanup on Windows, because on Linux the cleanup code was never triggered when I tried to test this. Apparently, on Linux gpgtar already takes care of removing the file if the operation is canceled.

This did not fix the issue on Windows.

[3376] gpg.qgpgme: sign_encrypt Removing output file "C:/Users/g10code.WIN-TEST3/Documents/tiefer_test.tar.gpg" after error or cancel
[3376] gpg.qgpgme: sign_encrypt Removing output file "C:/Users/g10code.WIN-TEST3/Documents/tiefer_test.tar.gpg" failed
ebo moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Sep 25 2023, 8:31 AM

it was decided to write the encrypted archive with ending .part and only rename it at the end. In this way the users can't think they have a valid encrypted archive

ikloecker changed the task status from Open to Testing.Oct 27 2023, 4:17 PM

We now use a temporary .part files when creating the archive. On success, they are renamed. Otherwise, they are removed (if possible).

ikloecker moved this task from Backlog to WiP on the vsd32 board.
ebo moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Nov 9 2023, 4:07 PM
ebo moved this task from WiP to QA on the vsd32 board.
ebo moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Nov 10 2023, 11:11 AM
ebo moved this task from QA to WiP on the vsd32 board.

With VS-Desktop-3.1.90.277-Beta the temporary file ends with .part now and is renamed properly when the job ends successfully.
But it is not removed when the job is aborted.

Does this count as solved?

We now use a temporary .part files when creating the archive. On success, they are renamed. Otherwise, they are removed (if possible).

I would say no, because the "Otherwise, they are removed (if possible)" is not solved. And if you abort a job it should be possible or we need to make it possible to remove the file.

If you cancel a download the part file is also removed. If the browser crashes, the part file is not removed. That is the behavior I would expect here, too.

ebo changed the task status from Testing to Open.Nov 13 2023, 3:14 PM
ebo assigned this task to ikloecker.
ebo moved this task from QA to WiP on the vsd32 board.

Exactly. If possible. Kleopatra tries, but it's not able to remove the file. Because some process in the background keeps it open.

Then we need to kill it with fire! :) Or maybe some context is still open at the time that keeps the process alive? I could investigate on windows. But on linux it might be easier to just breakpoint kleo right before the delete and do an lsof on the file? even though on linux the deletion would likely succeed.

Some observations on Linux:

  • If I cancel sign&encrypt archive while the encryption is still running then Kleopatra removes the .part file. I didn't see a running gpgtar or gpg process after I canceled.
  • If I cancel sign&encrypt archive by canceling the pinentry (asking for the password of the signing key) then the gpgtar and gpg processes keep running for a short time and Kleopatra still shows the progress. Eventually Kleopatra shows an error (General Error) instead of simply closing the window (-> T6575: gpgtar: General Error is emitted instead of more specific error codes). In this case Kleopatra didn't (have to) remove the .part file because it was already gone.
ikloecker changed the task status from Open to Testing.Nov 15 2023, 11:59 AM

Hopefully fixed for good.

works, the .part file is deleted on Windows, too, VS-Desktop-3.1.90.287-Beta

(It takes maybe 10 second before the file disappears when you hit F5 in the explorer)

ebo edited projects, added vsd32 (vsd-3.2.0); removed vsd32.
In T6584#179021, @ebo wrote:

(It takes maybe 10 second before the file disappears when you hit F5 in the explorer)

Makes sense because we try to delete the file every 10 seconds:

static const auto timeout = std::chrono::seconds{10};