From seth at userprimary.net  Sat Mar 20 22:03:18 2010
From: seth at userprimary.net (Seth Falcon)
Date: Sat, 20 Mar 2010 14:03:18 -0700
Subject: [Rd] list_files() memory corruption?
In-Reply-To: <e63850351003201336l76331245h421db8aeeeafe67d@mail.gmail.com>
References: <e63850351003121637rc13612et6fbd55fd453757f7@mail.gmail.com>	
	<4B9EF5D2.8010107@userprimary.net>	
	<e63850351003152037k1dbde4c1o2ba9c51ecd02fb4d@mail.gmail.com>	
	<4B9F0585.409@userprimary.net>	
	<e63850351003170716h23e60328oce0d6ed7f61f45d7@mail.gmail.com>	
	<4BA1067F.4030700@userprimary.net>	
	<e63850351003171059s180f4745qb88444966b98c189@mail.gmail.com>
	<e63850351003201336l76331245h421db8aeeeafe67d@mail.gmail.com>
Message-ID: <4BA53816.5050108@userprimary.net>

On 3/20/10 1:36 PM, Alistair Gee wrote:
> I fixed my build problems. I also noticed that my patch wasn't
> correct, so I have attached a new version.
> 
> This fix still grows the vector by doubling it until it is big enough,
> but the length is reset to the correct size at the end once it is
> known.
> 
> This fix differs from the existing fix in subversion in the following scenario:
> 
> 1.Create file Z in directory with 1 other file named Y
> 2. Call dir() to retrieve list of files.
> 3. dir() counts 2 files.
> 4. While dir() is executing, some other process creates file X in the directory.
> 5. dir() retrieves the list of files, stopping after 2 files. But by
> chance, it retrieves files X and Y (but not Z).
> 6. dir() returns files X and Y, which could be misinterpreted to mean
> that file Z does not exist.
> 
> In contrast, with the attached fix, dir() would return all 3 files.

I think the scenario you describe could happen with either version.
Once you've read the files in a directory, all bets are off.  Anything
could happen between the time you readdir() and return results back to
the user.  I agree, though, that avoiding two calls to readdir narrows
the window.

> Also, the existing fix in subversion doesn't seem to handle the case
> where readdir() returns fewer files than was originally counted as it
> doesn't decrease the length of the vector.

Yes, that's a limitation of the current fix.

Have you run 'make check-devel' with your patch applied?  Have you run
any simple tests for using dir() or list.files() with recursive=TRUE on
a reasonably large directory and compared times and memory use reported
by gc()?

It is often the case that writing the patch is the easy/quick part and
making sure that one hasn't introduced new infelicities or unintended
behavior is the hard part.

I will try to take another look at your latest patch.

+ seth



[Rd] list_files() memory corruption?
Seth Falcon