On Tue, Nov 08 2022, Phillip Wood wrote: > On 04/11/2022 14:50, Phillip Wood wrote: >> On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote: >>> When "read_strategy_opts()" is called we may have populated the >>> "opts->strategy" before, so we'll need to free() it to avoid leaking >>> memory. >> Where is the previous value coming from? I guess it may be the >> config but otherwise I'm confused. > > Having looked a bit more at this I think this is the wrong fix. The > reason we're overwriting the existing value is that we're reading some > of the state files twice. I think the only way to get to this point is > to go through a code path that calls rebase.c:read_basic_state() which > already populates these options via a later call to > rebase.c:get_replay_opts(). I think the correct fixes looks something > like the diff below. > > I have also looked at the cherry-pick/revert case and I think that > we're leaking opts->strategy (and probably some others) when running > > git cherry-pick --continue > > after > > git -c pull.twohead=recursive cherry-pick -s ort <some commits> Related: My just-sent [1]. For this one though, I have a v2 re-roll prepared, in which I explained why the leak is happening (I'll send this soon to the list, but for now): https://github.com/avar/git/commit/243ab74120b > I'm not sure what your strategy has been with the fixes in this series > but we're never going to have 100% coverage of all the option > combinations for rebase & cherry-pick so I think it is helpful to > treat these LSAN reports as a starting point for looking into why the > leak is occurring and also look for similar leaks. For this series I'm aiming to solve some common leaks, and increase our test coverage. Per [1] the destruction in this area is quite messy, and we should do it better. But that's a much larger fix. I think in the meantime adding this free() is the bets way to make incremental progress here, and (an the commit linked above notes) it's consistent with existing patterns. 1. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@xxxxxxxxxxxxxxxxxxx/
