Re: [patch] concurrency fixes for the fuse daemon

[Hans Petter Jansson <hpj novell com>]

On Sun, 2008-04-20 at 15:38 -0400, David Zeuthen wrote:

> After some more testing I realized we're leaking the reference. Which
> means the same stream will get reused and this fails if the backend
> doesn't support seeking (for example the archive backend; still worked
> great with backends that support seeking e.g. cdda, sftp, gphoto2). With
> the old patch this would happen on a fresh mount
> 
> $ file .gvfs/f9-20080319.iso/README 
> .gvfs/f9-20080319.iso/README: UTF-8 Unicode English text
> 
> $ file .gvfs/f9-20080319.iso/README 
> .gvfs/f9-20080319.iso/README: ERROR: cannot read `.gvfs/f9-20080319.iso/README' (Operation not supported)
> 
> The attached patch fixes that by unreffing the FileHandle in
> vfs_release() before freeing it.

Ok, if you've tested this version of the patch and it works, I think it
should be good to commit.

> Hmm.. I wonder if the correct fix
> involves not sharing FileHandle's at all? I mean, we only use it for
> stream operations anyway; doesn't seem we gain a lot by sharing these.

That leaves the question of how we detect when an open file gets
renamed. The shared table lets us redirect subsequent I/O on the open
file descriptor to the renamed file, emulating Unix semantics. There are
also special cases for unlink and ftruncate.

I'd rather get rid of that code if it's unnecessary, though. Does GVFS
handle all that for us internally?

[Looking at the code, I think I'm seeing an obvious bug in
reindex_file_handle_for_path() which would make rename-while-open crash
the daemon. So that would have to be fixed if we keep the global table.]

-- 
Hans Petter