Calling finished() after error() in KIO slaves causes data corruption and crashes

Vlad vladc6 at
Tue Mar 18 22:06:42 GMT 2008


--- David Faure <faure at> wrote:
> On Tuesday 18 March 2008, Vlad wrote:
> > Makes sense, but I think the documentation for SlaveBase::error()
> > should be stricter, such as:
> > 
> > "This also finishes the job, so you must not call finished after
> > calling this."
> > 
> > I'll update this message if that's OK. I'll also add a comment to
> > SlaveBase::finished().
> Yes please.
> It should be enough in SlaveBase to have a member var with three
> states:
> enum { Idle, InsideMethod, FinishedCalled, ErrorCalled } m_state;
> d->m_state would be set to InsideMethod before calling get(),
> and in both finished() you would say
>  if (d->m_state == FinishedCalled) {
>   kWarning() << "finished() called twice!";
>  } else if (d->m_state == ErrorCalled) {
>   kWarning() << "finished() called after error()!";
>  }
> and similar in error().
> And when get() returns, you set the state to Idle again. You could
> even check
> that the state was FinishedCalled or ErrorCalled.
> The point is that get() (and all other operations) is a blocking
> method call. So there's no
> need for any job ID: you know that it's done when it returns, and
> there can only be one
> operation at a time.

Thank you for this suggestion. I'm attaching a patch where
slavebase.cpp and slavebase.h are modified accordingly, and I've
verified that warnings are printed whenever the kio_smb tries to call
finished() after error().

Unfortunately, I'm having trouble committing the patch due to my ISP's
firewall, so if it looks OK, please feel free to commit it.


Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ
-------------- next part --------------
A non-text attachment was scrubbed...
Name: slavebase.diff
Type: text/x-patch
Size: 10652 bytes
Desc: 3785055382-slavebase.diff
URL: <>

More information about the kde-core-devel mailing list