Unexpected behavior in KIO::MimeTypeFinderJob

[David Faure]

On mercredi 7 août 2024 15:53:37 heure d’été d’Europe centrale Harald Sitter 
wrote:
> On Wed, Aug 7, 2024 at 12:10 PM David Faure <faure at kde.org> wrote:
> > On mercredi 7 août 2024 11:54:20 heure d’été d’Europe centrale David Faure
> > 
> > wrote:
> > > On mercredi 7 août 2024 08:30:43 heure d’été d’Europe centrale Stefano
> > > Crocco
> > > 
> > > wrote:
> > > > Hello to everyone,
> > > > while investigating a bug in Konqueror, I just found what in my
> > > > opinion is
> > > > an unexpected behavior of KIO::MimeTypeFinderJob. If I'm reading the
> > > > code
> > > > correctly, when it uses KIO::get() to determine the mimetype [1], it
> > > > lets
> > > > the TransferJob go on even after it detected the mimetype.
> > > > 
> > > > The documentation for KIO::get() states:
> > > > "Special case: if you want to determine the MIME type of the file
> > > > first,
> > > > and then read it with the appropriate component, you can still use a
> > > > KIO::get() directly. When that job emits the mimeType signal, (which
> > > > is
> > > > guaranteed to happen before it emits any data), put the job on hold".
> > > > Since the task of MimeTypeFinderJob is finding the mimetype of the
> > > > URL, I
> > > > expected it would put the job on hold as soon as it has determined the
> > > > mimetype, coherently with the KIO::get() documentation.
> > > 
> > > Right, that's what KRun::foundMimeType was doing, and I did that in
> > > MimeTypeFinderJob initially.
> > > 
> > > But see commit 621585f0e818cd9f75cdd3bcc8665da7ac708283
> > > and bug 452729. Seems weird. Talk to Harald ;)
> > 
> > Of course in KF5 it was a bit different because another process could
> > resume the slave on hold, made available by klauncher. AFAIK nowadays
> > that's gone so resuming the slave on hold only works if the *same*
> > process is going to fetch that URL. I guess that's the issue. If another
> > process gets started instead (to open that URL in e.g. an image viewer),
> > that slave on hold stays there forever. We need a way to discard it then,
> > somehow.
> 
> I don't see the problem. I mean, the docs are wrong, but beyond that?
> 
> MimeTypeFinderJob is a KCompositeJob. It subjob()s the TransferJob.
> subjob calls setParent. When the MimeTypeFinderJob gets deleted the
> TransferJob gets deleted and that triggers the Scheduler to terminate
> the Worker process.

You're saying "the current code doesn't leak anything". We all agree on that.

The problem with the current code is that it completely removed the feature of 
reusing the same worker for determining the mimetype and for actually fetching 
the data, when both happen in the same process.

Your commit removed that because in the case where both do *NOT* happen in the 
same process, then the slave was kept on hold forever.

So to make everyone happy, we need a way to put a slave on hold, reuse it if 
doing a get() in the same process, and cleaning it up otherwise.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20240807/a5888354/attachment.sig>