Race condition with data loss in ReadWritePart

[Martin Pley]

Am Sonntag 24 Februar 2008 19:36:18 schrieb Ingo Klöcker:
> On Sunday 24 February 2008, Martin Pley wrote:
> > Am Sonntag, 24. Februar 2008 13:18:17 schrieb Ingo Klöcker:
> > > On Saturday 23 February 2008, Martin Pley wrote:
> > > > > I noticed a race condition with data loss in ReadWritePart:
> > > > >
> > > > > Try to open a non-local-file from a slow or offline server.
> > > > > Then, instead of waiting for the file, just open another local
> > > > > file. Now, when you delete the part, the file is deleted(!).
> > > > >
> > > > > If I see it correctly, this is because m_bTemp is set to "true"
> > > > > while opening the remote file. And when you open the local file
> > > > > it will not be set to false again.
> > > >
> > > > It's actually not a race condition. I reimplemented
> > > > ReadWritePart::closeURL() without setting m_bTemp to false. And
> > > > when I delete the part, ~ReadOnlyPart calls
> > > > ReadOnlyPart::closeURL(), which deletes the file.
> > > >
> > > > Perhaps one should add a warning to the documentation of
> > > > closeURL().
> > >
> > > There's no need for adding a warning. We just need to call
> > > closeURL() from ~ReadWritePart.
> >
> > IMO this is not a good idea. You would get a
> > save-or-ignore-messagebox in the destructor. But saveFile() might not
> > exist anymore because the destuctor of the derived class was already
> > called. One should be carefull with calling virtual methods from
> > constructor or destructor.
>
> True.
>
> > However, in my case this would not help at all.
>
> After having a quick look at the code to me the problem seems to be that
> m_bTemp is not set to false in ReadOnlyPart::openURL() in
>
>   if ( d->m_url.isLocalFile() )
>   { ... }
>
> while it's set to true in the corresponding else-branch. This would help
> in your case, right?

That would have helped to avoid data loss, yes.

But how about writing a ReadWritePart::openURL(), which calls 
ReadWritePart::closeURL() first? Then ReadOnlyPart::openURL() may just call 
ReadOnlyPart::closeURL(). What do you think?

Kind regards
Martin