From alistair.gee at gmail.com  Wed Mar 17 18:59:06 2010
From: alistair.gee at gmail.com (Alistair Gee)
Date: Wed, 17 Mar 2010 10:59:06 -0700
Subject: [Rd] list_files() memory corruption?
In-Reply-To: <4BA1067F.4030700@userprimary.net>
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>
Message-ID: <e63850351003171059s180f4745qb88444966b98c189@mail.gmail.com>

On Wed, Mar 17, 2010 at 9:42 AM, Seth Falcon <seth at userprimary.net> wrote:
> On 3/17/10 7:16 AM, Alistair Gee wrote:
>> Yes. I had noticed that R occasionally segfaults (especially when I
>> run many concurrent R processes), so I used valgrind to log every use
>> of R. In the valgrind logs, I tracked the problem to list_files().
>>
>> I attached a patch to platform.c (for trunk). Unfortunately, I am
>> having trouble building R from the subversion trunk--it is taking a
>> very long time decompressing/installing the recommended packages--so I
>> haven't been able to verify the fix yet. But my version of platform.c
>> does compile, and it does simplify the code b/c count_files() is no
>> longer needed.
>
> Hmm, I see that you "grow" the vector containing filenames by calling
> lengthgets and doubling the length. ?I don't see where you cleanup
> before returning -- seems likely you will end up returning a vector that
> is too long.
>
> And there are some performance characteristics to consider in terms of
> both run time and memory profile. ?Does making a single pass through the
> files make up for the allocations/data copying that result from
> lengthgets? ?Is it worth possibly requiring twice the memory for the
> worst case?
>
> + seth
>
>
>
>
>
Sorry, I left out a call to shorten the vector (via final call to
lengthgets()). See new patch.

BTW, I modeled this code after code I found in the RODBC package that
handles rows returned from a database query. I don't know if this is a
typical approach to reallocation. Maybe there is a better way of
extending the vector, though wouldn't most of the memory usage be in
the strings (of the filenames) rather than the STRSXP vector itself?

Anyway, I'm offering this (untested) fix as it handles both
directories that have grown and directories that have shrunk, so that
the length of the vector is correct in both cases.

--
-------------- next part --------------
A non-text attachment was scrubbed...
Name: platform.c.patch
Type: text/x-patch
Size: 4328 bytes
Desc: not available
URL: <https://stat.ethz.ch/pipermail/r-devel/attachments/20100317/41cea3e7/attachment.bin>


[Rd] list_files() memory corruption?
Alistair Gee