git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 0/20] allowing struct lock_file to be deleted

From
Jeff King <peff@peff.net>
Date
Sep 5, 2017, 12:13 UTC
Message-ID
<20170905121353.62zg3mtextmq5zrs@sigill.intra.peff.net>

Since the beginning of time, our code base has had the rule that the storage for a "struct lock_file" can never go out of scope once it has been used. This leads to lots of intentional leaks of these structs.

I think it's unlikely that any of the leaks produces truly terrible behavior, simply because we tend to do a limited number of operations from a given program (and the structs aren't that big to begin with). But they make finding _actual_ leaks much harder by polluting leak-checker output.

And I have to admit that they've just always bugged me.

This series make it possible to free them (and in IMHO makes it harder to misuse the API in general). I also found a number of interesting corner cases that turned out to be bugs in the existing code (albeit quite minor). I've floated the existing fixes to the front, followed by some preparatory cleanups. The meat is in patches 17 and 18, and then the last few patches use the new system to clean up some leaks.

There are almost certainly more leaks that can be cleaned. I didn't do an exhaustive search, but just covered all the ones triggered by t0000 and t0001 (which I've been doggedly trying to get to run clean in a leak-checker; more on that in a moment).

  [01/20]: write_index_as_tree: cleanup tempfile on error
  [02/20]: setup_temporary_shallow: avoid using inactive tempfile
  [03/20]: setup_temporary_shallow: move tempfile struct into function
  [04/20]: verify_signed_buffer: prefer close_tempfile() to close()
  [05/20]: always check return value of close_tempfile
  [06/20]: tempfile: do not delete tempfile on failed close
  [07/20]: lockfile: do not rollback lock on failed close
  [08/20]: tempfile: prefer is_tempfile_active to bare access
  [09/20]: tempfile: handle NULL tempfile pointers gracefully
  [10/20]: tempfile: replace die("BUG") with BUG()
  [11/20]: tempfile: factor out activation
  [12/20]: tempfile: factor out deactivation
  [13/20]: tempfile: robustify cleanup handler
  [14/20]: tempfile: release deactivated strbufs instead of resetting
  [15/20]: tempfile: use list.h for linked list
  [16/20]: tempfile: remove deactivated list entries
  [17/20]: tempfile: auto-allocate tempfiles on heap
  [18/20]: lockfile: update lifetime requirements in documentation
  [19/20]: ref_lock: stop leaking lock_files
  [20/20]: stop leaking lock structs in some simple cases
 builtin/gc.c               |   8 +-
 builtin/reset.c            |   6 +-
 builtin/update-index.c     |  11 +--
 cache-tree.c               |  37 ++++----
 config.c                   |  24 ++---
 credential-cache--daemon.c |   5 +-
 diff.c                     |  15 ++-
 gpg-interface.c            |  16 ++--
 list.h                     |  38 ++++++++
 lockfile.c                 |   7 +-
 lockfile.h                 |  58 ++++++------
 read-cache.c               |  32 ++++---
 refs/files-backend.c       |  50 +++++-----
 refs/packed-backend.c      |  14 +--
 shallow.c                  |  16 ++--
 tempfile.c                 | 232 +++++++++++++++++++++++----------------------
 tempfile.h                 | 123 +++++++++++-------------
 trailer.c                  |   6 +-
 18 files changed, 358 insertions(+), 340 deletions(-)
-Peff
Next: Jeff King
Message 1 of 21 in “allowing struct lock_file to be deleted”
  1. 0/20 allowing struct lock_file to be deletedJeff King, Sep 5, 2017
  2. 01/20 write_index_as_tree: cleanup tempfile on errorJeff King, Sep 5, 2017
  3. 02/20 setup_temporary_shallow: avoid using inactive tempfileJeff King, Sep 5, 2017
  4. 03/20 setup_temporary_shallow: move tempfile struct into functionJeff King, Sep 5, 2017
  5. 04/20 verify_signed_buffer: prefer close_tempfile() to close()Jeff King, Sep 5, 2017
  6. 05/20 always check return value of close_tempfileJeff King, Sep 5, 2017
  7. 06/20 tempfile: do not delete tempfile on failed closeJeff King, Sep 5, 2017
  8. 07/20 lockfile: do not rollback lock on failed closeJeff King, Sep 5, 2017
  9. 08/20 tempfile: prefer is_tempfile_active to bare accessJeff King, Sep 5, 2017
  10. 09/20 tempfile: handle NULL tempfile pointers gracefullyJeff King, Sep 5, 2017
  11. 10/20 tempfile: replace die("BUG") with BUG()Jeff King, Sep 5, 2017
  12. 11/20 tempfile: factor out activationJeff King, Sep 5, 2017
  13. 12/20 tempfile: factor out deactivationJeff King, Sep 5, 2017
  14. 13/20 tempfile: robustify cleanup handlerJeff King, Sep 5, 2017
  15. 14/20 tempfile: release deactivated strbufs instead of resettingJeff King, Sep 5, 2017
  16. 16/20 tempfile: remove deactivated list entriesJeff King, Sep 5, 2017
  17. 17/20 tempfile: auto-allocate tempfiles on heapJeff King, Sep 5, 2017
  18. 18/20 lockfile: update lifetime requirements in documentationJeff King, Sep 5, 2017
  19. 19/20 ref_lock: stop leaking lock_filesJeff King, Sep 5, 2017
  20. 20/20 stop leaking lock structs in some simple casesJeff King, Sep 5, 2017
  21. 15/20 tempfile: use list.h for linked listJeff King, Sep 5, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.