Stefan Küng wrote:
> Stefan Fuhrmann wrote:
> > Hi there,
> >
> > some days ago, a coworker had TSVNCache go berserk and
> > finally eating 3GB of memory (x64). That gave me sufficient
> > motivation to search the code for improper memory management.
> >
> > r15974 fixes a number of obvious ones and I merged that code
> > also into 1.6.x. r15975-7 try to make other parts more robust.
> >
> > Since TSVNCache is notoriously hard to test, it is very possible
> > that I broke something. If you encounter any problem with the
> > next nightlies, please tell us.
> >
> > Stefan, could you review my changes?
>
> Had to change some of your code:
> * didn't compile
Oops .. I really want to apologize for that! I thought, I had compiled
just before committing ... :(
> * had to remove most of the auto_ptr use you introduced because they
> threw runtime exceptions
I finally figured out how to debug TSVNCache - it wasn't that
hard after all. And I fixed all auto_ptr usage issues locally but
will not commit them right row (see below).
> * your 'paranoia' for-loop wouldn't actually do anything?
It does. In particular, it will clear the whole string, if it is not
0-terminated. Just terminating it at the end of the buffer may
make the result accidentally matching a valid path.
In r15991, I enhanced my code a bit. It will now clear all
bytes beyond the paths terminating 0. Also, it enables
strict memory checking in debug mode.
> But due to your changes, I found another bug which I could fix.
And there is a lot more going on in TSVNCache:
* tear-down sequence is broken: triggers debug exception
in the CRWSection destructor.
* someone is writing to deleted data, possibly in
CDirectoryWatcher::CloseInfoMap
Because of the latter, I hold off my changes until the root cause
has been found. My problem is that this seems to be very dated
code and is hard to understand. For instance CloseInfoMap takes
a handle parameter and will remove the respective path from
other collections. But the remaining entries will be deleted without
such special handling (and there are often more than one entries).
Why?
-- Stefan^2.
Received on 2009-03-29 21:19:33 CEST