[vtk-developers] [Paraview-developers] change to vtkExodusIIReaderPrivate::ResetCache()

[David Thompson]

> I just found an issue where the polygon code was assuming the cache  
> was
> valid after a second call (which destroyed the array from the previous
> call).
>
> I added Register/Unregister to keep the array.  Which may not really  
> be
> any safer.

That is exactly what should be done.

> What is the proper response?  Call out to GetCacheOrRead each
> time an array is used?

What do you mean by "call out to GetCacheOrRead"? That is how the  
array was obtained was it not? The cache should hold on to it until  
the next call to GetCacheOrRead, at which point it *may* release its  
reference. If someone else holds a reference at that point then the  
object stays around. Otherwise, kaput!

> I haven't really carefully audited the code, but in general it does  
> look
> like GetCacheOrRead is called once and then that array is used  
> immediately
> after, but I could see this cropping up again...

Yes. I tried to document GetCacheOrRead as best I could, but there is  
that danger. I think the more insidious (or at least more difficult to  
detect) problem would be where a function calls another function which  
in turn calls GetCacheOrRead. That is why you should always  
immediately Register() any object returned by the cache until you're  
done with it.

	D

>
> On 4/25/11 12:21 PM, "David Thompson" <dcthomp at sandia.gov> wrote:
>
>>> From a pull from master some weeks ago, the previous version of this
>>> method looked like this:
>>> ...
>>> When reading in an exodus file now, the code disappears into the
>>> reader and never finishes (maybe it would eventually, but after a
>>> long while I kill it).  Adding back in the line to set the cache
>>> capacity to 128 resolves this issue.  I expect that the line was
>>> removed for a reason.  Did removing the line resolve a problem for
>>> someone else?  Is anyone else having difficulty with the exodus
>>> reader as a result of this change?  Or is it something specific to
>>> my exodus file/runtime environment?
>>
>> Hi Pat,
>>
>> The line was removed to address a problem Alan had reported when
>> opening many small files with the parallel reader... the sum of the
>> caches (one from each sub-reader owned by the parallel reader) was
>> growing too large. I haven't heard whether anyone else is having
>> problems but Alan reported it fixed his issue. If I had to guess, I
>> would say there's some code in there relying on the cache holding on
>> to a reference to a vtkDataArray when it should not (even with a
>> 128MiB cache, there was never any guarantee). If so, valgrind should
>> pick up on the reference to freed memory. Can you run your code
>> through valgrind and see if it complains?
>>
>>   David
>>
>>
>> _______________________________________________
>> Paraview-developers mailing list
>> Paraview-developers at paraview.org
>> http://public.kitware.com/mailman/listinfo/paraview-developers
>>
>
>
> _______________________________________________
> Paraview-developers mailing list
> Paraview-developers at paraview.org
> http://public.kitware.com/mailman/listinfo/paraview-developers
>