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

[PATCH 00/11] various lockfile-leaks and -fixes

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Oct 1, 2017, 14:56 UTC
Message-ID
<cover.1506862824.git.martin.agren@gmail.com>

A recent series allowed `struct lock_file`s to be freed [1], so I wanted to get rid of the "simple" leaks of this kind. I found a couple of lock-related cleanups along the way and it resulted in this series. It feels a bit scary at eleven patches -- especially as this is about locking -- but I hope I've managed to put this into understandable and reviewable patches. Reviews, thoughts, opinions welcome, as always.

1-2

Instead of allocating and leaking `struct lock_file`s, initialize them on the stack. These instances were identified by simple grepping.

3-4
Documentation fixes for lockfile and temporary file APIs.
5-7

No need to represent the same information twice. No need for the caller to provide a (static) lock for us now that we can safely manage our own.

8-11

Error-handling in read-cache.c was a bit lacking and over-eager at the same time. Sometimes we'd leave the lock open when we should commit it, sometimes we'd roll it back when we should only close it. We'd also crash under rare circumstances. Patch 9 addresses a bug in the API which most likely hasn't hit anyone and maybe never would, but if we don't have the functionality, I don't think we should pretend like we do.

Martin
[1] https://public-inbox.org/git/20170905121353.62zg3mtextmq5zrs@sigill.intra.peff.net/
Martin Ågren (11):
  sha1_file: do not leak `lock_file`
  treewide: prefer lockfiles on the stack
  lockfile: fix documentation on `close_lock_file_gently()`
  tempfile: fix documentation on `delete_tempfile()`
  cache-tree: simplify locking logic
  apply: move lockfile into `apply_state`
  apply: remove `newfd` from `struct apply_state`
  cache.h: document `write_locked_index()`
  read-cache: require flags for `write_locked_index()`
  read-cache: don't leave dangling pointer in `do_write_index()`
  read-cache: roll back lock on error with `COMMIT_LOCK`
 apply.c            | 25 ++++++++-----------------
 apply.h            |  8 +++-----
 builtin/am.c       | 27 ++++++++++++---------------
 builtin/apply.c    |  4 +---
 builtin/checkout.c | 14 ++++++--------
 builtin/clone.c    |  7 +++----
 builtin/diff.c     |  7 +++----
 builtin/difftool.c |  1 -
 cache-tree.c       | 12 ++++--------
 cache.h            | 19 +++++++++++++++++++
 config.c           | 17 ++++++++---------
 git-compat-util.h  |  7 ++++++-
 lockfile.h         |  4 ++--
 merge-recursive.c  |  6 +++---
 merge.c            |  8 +++-----
 read-cache.c       | 26 ++++++++++++++------------
 sequencer.c        |  1 -
 sha1_file.c        | 16 +++++++---------
 tempfile.h         |  8 ++++----
 wt-status.c        |  8 ++++----
 20 files changed, 110 insertions(+), 115 deletions(-)
-- 
2.14.1.727.g9ddaf86
Next: Martin Ågren
Message 1 of 69 in “various lockfile-leaks and -fixes”
  1. 00/11 various lockfile-leaks and -fixesMartin Ågren, Oct 1, 2017
  2. 01/11 sha1_file: do not leak `lock_file`Martin Ågren, Oct 1, 2017
  3. Jeff KingOct 2, 2017
  4. Martin ÅgrenOct 2, 2017
  5. 02/11 treewide: prefer lockfiles on the stackMartin Ågren, Oct 1, 2017
  6. Junio C HamanoOct 2, 2017
  7. Martin ÅgrenOct 2, 2017
  8. Jeff KingOct 2, 2017
  9. 03/11 lockfile: fix documentation on `close_lock_file_gently()`Martin Ågren, Oct 1, 2017
  10. Jeff KingOct 2, 2017
  11. 04/11 tempfile: fix documentation on `delete_tempfile()`Martin Ågren, Oct 1, 2017
  12. Jeff KingOct 2, 2017
  13. 05/11 cache-tree: simplify locking logicMartin Ågren, Oct 1, 2017
  14. Junio C HamanoOct 2, 2017
  15. Jeff KingOct 2, 2017
  16. 06/11 apply: move lockfile into `apply_state`Martin Ågren, Oct 1, 2017
  17. Jeff KingOct 2, 2017
  18. 07/11 apply: remove `newfd` from `struct apply_state`Martin Ågren, Oct 1, 2017
  19. Jeff KingOct 2, 2017
  20. 08/11 cache.h: document `write_locked_index()`Martin Ågren, Oct 1, 2017
  21. 09/11 read-cache: require flags for `write_locked_index()`Martin Ågren, Oct 1, 2017
  22. Junio C HamanoOct 2, 2017
  23. Martin ÅgrenOct 2, 2017
  24. Martin ÅgrenOct 2, 2017
  25. Jeff KingOct 2, 2017
  26. 10/11 read-cache: don't leave dangling pointer in `do_write_index()`Martin Ågren, Oct 1, 2017
  27. Jeff KingOct 2, 2017
  28. Jeff KingOct 2, 2017
  29. 11/11 read-cache: roll back lock on error with `COMMIT_LOCK`Martin Ågren, Oct 1, 2017
  30. Junio C HamanoOct 2, 2017
  31. Junio C HamanoOct 2, 2017
  32. Jeff KingOct 2, 2017
  33. Junio C HamanoOct 2, 2017
  34. Martin ÅgrenOct 2, 2017
  35. Junio C HamanoOct 3, 2017
  36. 00/12 Re: various lockfile-leaks and -fixesMartin Ågren, Oct 5, 2017
  37. 01/12 sha1_file: do not leak `lock_file`Martin Ågren, Oct 5, 2017
  38. Junio C HamanoOct 6, 2017
  39. 02/12 treewide: prefer lockfiles on the stackMartin Ågren, Oct 5, 2017
  40. 03/12 lockfile: fix documentation on `close_lock_file_gently()`Martin Ågren, Oct 5, 2017
  41. 05/12 checkout-index: simplify locking logicMartin Ågren, Oct 5, 2017
  42. Junio C HamanoOct 6, 2017
  43. 06/12 cache-tree: simplify locking logicMartin Ågren, Oct 5, 2017
  44. 07/12 apply: move lockfile into `apply_state`Martin Ågren, Oct 5, 2017
  45. 08/12 apply: remove `newfd` from `struct apply_state`Martin Ågren, Oct 5, 2017
  46. 10/12 read-cache: drop explicit `CLOSE_LOCK`-flagMartin Ågren, Oct 5, 2017
  47. Junio C HamanoOct 6, 2017
  48. Martin ÅgrenOct 6, 2017
  49. 11/12 read-cache: leave lock in right state in `write_locked_index()`Martin Ågren, Oct 5, 2017
  50. Junio C HamanoOct 6, 2017
  51. Martin ÅgrenOct 6, 2017
  52. Junio C HamanoOct 6, 2017
  53. Martin ÅgrenOct 6, 2017
  54. 00/12 Re: various lockfile-leaks and -fixesMartin Ågren, Oct 6, 2017
  55. 01/12 sha1_file: do not leak `lock_file`Martin Ågren, Oct 6, 2017
  56. 02/12 treewide: prefer lockfiles on the stackMartin Ågren, Oct 6, 2017
  57. 04/12 tempfile: fix documentation on `delete_tempfile()`Martin Ågren, Oct 6, 2017
  58. 06/12 cache-tree: simplify locking logicMartin Ågren, Oct 6, 2017
  59. 05/12 checkout-index: simplify locking logicMartin Ågren, Oct 6, 2017
  60. 08/12 apply: remove `newfd` from `struct apply_state`Martin Ågren, Oct 6, 2017
  61. 09/12 cache.h: document `write_locked_index()`Martin Ågren, Oct 6, 2017
  62. 10/12 read-cache: drop explicit `CLOSE_LOCK`-flagMartin Ågren, Oct 6, 2017
  63. 12/12 read_cache: roll back lock in `update_index_if_able()`Martin Ågren, Oct 6, 2017
  64. 11/12 read-cache: leave lock in right state in `write_locked_index()`Martin Ågren, Oct 6, 2017
  65. 03/12 lockfile: fix documentation on `close_lock_file_gently()`Martin Ågren, Oct 6, 2017
  66. 07/12 apply: move lockfile into `apply_state`Martin Ågren, Oct 6, 2017
  67. 09/12 cache.h: document `write_locked_index()`Martin Ågren, Oct 5, 2017
  68. 12/12 read_cache: roll back lock in `update_index_if_able()`Martin Ågren, Oct 5, 2017
  69. 04/12 tempfile: fix documentation on `delete_tempfile()`Martin Ågren, Oct 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.