Re: [PATCH] builtin/init-db: preemptively clear repo_fmt to avoid leaks

Hi Andrzej,

On Thu, 6 May 2021, Andrzej Hunt wrote:

On 05/05/2021 21:28, Johannes Schindelin wrote:

quoted

On Wed, 5 May 2021, Andrzej Hunt via GitGitGadget wrote:

quoted

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 31b718259a64..b25147ebaf59 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c

@@ -512,6 +512,7 @@ int init_db(const char *git_dir, const char

*real_git_dir,
   		       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
   }

+	clear_repository_format(&repo_fmt);

I am afraid that this might not be correct, as t0410.27 experienced a
segmentation fault (see
https://github.com/git/git/pull/1018/checks?check_run_id=2511749719#step:5:2845
for the full details):

Thanks for spotting that. On further investigation this looks like a
preexisting issue on seen (my github PR was based on seen - in hindsight that
was probably not a good idea) - here's a CI run from seen this morning
exhibiting the same failure:
  https://github.com/git/git/runs/2515095446?check_suite_focus=true

To gain more confidence, I've rebased my patch onto next, and I no longer hit
any CI failures:
  https://github.com/ahunt/git/runs/2515309312?check_suite_focus=true
(I ran this on my fork because changing the base branch appears to have
confused the PR's CI configuration.)

Oh, I'm sorry! But it is great news that this is not _your_ patch's
problem (I had a brief look over it and wondered why it would cause this
segfault).

Nevertheless, I was still curious about what might be causing the failure in
the first place: it appears to only happen in CI with gcc on Linux, and I was
not able to repro on my machine using (tested with gcc-7.5 and gcc-10, with
seen from this morning):
  make CC=gcc-10 T=t0410-partial-clone.sh test

There are more settings used in the CI run, indeed. So yeah, it would be
good to debug this in an environment where it does fail.

I'm guessing that understanding this failure might require the help of someone
with an Ubuntu install to debug (unless there's some easy way of using
Github's Actions to run a bisect)?

As a matter of fact, there is a neat way in GitHub Actions to debug this:
the `action-tmate` Action. It installs and starts a `tmate` instance,
which is a `tmux` inside kind of a proxied SSH server.

This is what I usually do when I have time to track down such problems: I
edit `.github/workflows/main.yml` and push it into a branch in my fork.
The edits are essentially:

- I delete the `ci-config` job (because I know that I want to run this
  workflow and don't want to wait for that job to be queued and run)

- Obviously, this requires all the `needs: ci-config` and
  `if: needs.ci-config.outputs.enabled == 'yes'` lines to be deleted, too.

- I also delete the jobs, tasks, and matrix elements that are not
  necessary for my debugging session (in your case, that would be e.g. the
  `dockerized` job as well as the vectors in `regular` that are not
  `linux-gcc`).

- Then comes the most important part: after the
  `ci/print-test-failures.sh` task, I insert this task:

    - name: action-tmate
      if: failure()
      uses: mxschmitt/action-tmate@v3
      with:
        limit-access-to-actor: true

The `limit-access-to-actor` thing only applies if you uploaded your public
SSH key to your GitHub profile. Otherwise, anybody who monitors the run
could see the command-line to connect to your debugging instance and
interfere with you.

For details, see https://mxschmitt.github.io/action-tmate/

Thanks,
Dscho