Page MenuHome GnuPG

Kleopatra: Hangs when decrypting an archive on an USB Stick
Closed, DuplicatePublic

Description

High priority as this should really have been fixed for the last release.

To reproduce:

  • Mount a USB Stick
  • Create a folder with a file and a subfolder and add a file in the subfolder
  • Encrypt it
  • Decrypt it

-> Kleopatra becomes unresponsive and churns on a CPU core. This happens with only a gpgtar process running and no gnupg process.

It has also been reported that it hangs after successful decryption.

Event Timeline

aheinecke created this task.

So the problem is occuring when the output is finalized (which happens after the GpgME Decrypt Result is signalled). And when there are still bytes to write in line 332 https://dev.gnupg.org/source/kleo/browse/master/src/utils/output.cpp$332

m_proc is a normal QProcess here.
Even if I tried it with a waitforbyteswritten timeout of 1ms and afterwards called QApplication::processEvents it would just not write and kept reporting it had bytesToWrite.
I am pretty sure that back in 2013 when I added this that this worked somehow.

I have tried to explicitly call m_proc->closeWriteChannel(), still no writes even if the documentation says it would write. It is like the Process is suspended.

Even the code:

diff --git a/src/utils/output.cpp b/src/utils/output.cpp
index 1ad7ea7f..d0e6279c 100644
--- a/src/utils/output.cpp
+++ b/src/utils/output.cpp
@@ -319,23 +319,13 @@ public:
         return m_proc;
     }
     void doFinalize() override {
-        /*
-          Make sure the data is written in the output here. If this
-          is not done the output will be written in small chunks
-          trough the eventloop causing an unnecessary delay before
-          the process has even a chance to work and finish.
-          This delay is mainly noticeable on Windows where it can
-          take ~30 seconds to write out a 10MB file in the 512 byte
-          chunks gpgme serves. */
-        qCDebug(KLEOPATRA_LOG) << "Waiting for " << m_proc->bytesToWrite()
-        << " Bytes to be written";
-        while (m_proc->waitForBytesWritten(PROCESS_MAX_RUNTIME_TIMEOUT));
-
         if (!m_proc->isClosed())
         {
             m_proc->close();
         }
+        qCDebug(KLEOPATRA_LOG) << "Waiting for finished";
         m_proc->waitForFinished(PROCESS_MAX_RUNTIME_TIMEOUT);
+        qCDebug(KLEOPATRA_LOG) << "finished";
     }

Caused it to hang in the waitForFinished.

What works is if I switch to our Windowsprocessdevice which we have under better control and which uses blocking writes.


The WindowsProcessDevice was originally implemented to avoid the complexities of QProcess as we somehow lost blocks when reading tar input.

But before I commit this I would like to understand why QProcess does not write the last bytes. @ikloecker Could you take a look if you find some QProcess misuse there?

The first thing I noticed is that ProcessStdInOutput uses redirect_close<QProcess> as process class. redirect_close overrides QProcess::close, but it never calls the original close(). That's very dubious. Maybe it's not a problem, but there's no comment explaining why the original close() doesn't need to be called. Or maybe that's what is causing the hang in waitForFinished.

And does this really only occur with an archive on a USB stick? If yes, this makes me wonder if the USB stack is misbehaving because why should using a USB stick make a difference? Which OS are we talking about anyway? Windows?

Yes this is windows. With the USB Stick I think it makes it more reproducable that bytesToWrite will return something. We also had reports that this happened on Network drives so its probably just "some added delay or something where the Windows Kernel does not do buffering".

On Linux it might be reproducible if there are bytes left to be written, but I was unable to reproduce that. But QProcess on Linux behaves very differently anyway.

While I agree that the redirect_close<QProcess> seems a bit fishy, it is not relevant here because it is only called when the shared_ptr is released. And even if I call the QProcess::close directly.

@ikloecker the proper fix for this would be T5478 so you do not have to spend time looking for this, we will remove the Process Input / Outputs of Kleopatra altogether.

@ikloecker the proper fix for this would be T5478 so you do not have to spend time looking for this, we will remove the Process Input / Outputs of Kleopatra altogether.

+1

I had been wondering whether reimplementing the shells' pipe operator is a good idea.

ebo removed a project: Restricted Project.Apr 12 2023, 2:53 PM