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

[Vlad]

Hi,

--- David Faure <faure at kde.org> 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.

Thanks,
Vlad


      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_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: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080318/6b1eee4e/attachment.bin>