----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33623/#review81890 -----------------------------------------------------------
proton-c/src/tests/object.c <https://reviews.apache.org/r/33623/#comment132414> The pn_string_t type is mutable, so the hash you are setting up here has a lot of different keys all pointing to a single mutable string value. The reason this appears to manifest as a valgrind error is that your assertion checking the value associated with each key is failing since all the keys map to the same value. When this happens the map (and string value) are not freed. The relevant error though is not the memory leak but the assertion failure. This is a bit hard to spot in the test output since the valgrind output is noiser, but the process is also aborting with the assert failure. It occurs to me if we install a handler for the abort signal and print a stack trace then it would be a lot more obvious what is going on here. - Rafael Schloming On April 28, 2015, 6:35 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33623/ > ----------------------------------------------------------- > > (Updated April 28, 2015, 6:35 p.m.) > > > Review request for qpid, Alan Conway, Andrew Stitcher, and Rafael Schloming. > > > Bugs: PROTON-858 > https://issues.apache.org/jira/browse/PROTON-858 > > > Repository: qpid-proton-git > > > Description > ------- > > The internal map implementation for proton-c uses coalesced hashing, but the > current deletion algorithm does not take account of this. On deletion of > entries, the map can lose its ability to locate other entries by key. > > This patch is the simplest possible fix. It frees the deleted entry, makes > any previous entry that points to this tha tail of its chain, and then > relocates any entries in a chain following the deleted entry. Though I > believe this to be correct and safe, it does involve more work on some > deletions. The approach can be optimised further, either be reducing the > amount of chain that needs relocated, or by marking records as deleted in the > first instance and only rehashing later on some criteria. > > (I started off trying to do the latter, but it is more complicated and I > still haven't got my patch fully correct. When I get that working I'll post > that also. It is a more involved change though, so perhaps we may want to > consider this simpler approach for 0.9.1). > > > Diffs > ----- > > proton-c/src/object/map.c 1a758a1 > proton-c/src/tests/object.c 2b213c5 > > Diff: https://reviews.apache.org/r/33623/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
