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

Re: [PATCH 06/34] clone: release strbuf after use in remove_junk()

From
Jeff King <peff@peff.net>
Date
Sep 13, 2017, 12:56 UTC
Message-ID
<20170913125611.zdy3r5gijz5ohl2r@sigill.intra.peff.net>
In-Reply-To
<1c0918c7-8630-e545-6c34-ab03995797bb@web.de>
On Mon, Sep 11, 2017 at 11:40:05PM +0200, René Scharfe wrote:
Show 8 quoted lines
> > Yes, but I didn't want to touch each code site that creates a file,
> > which is a lot more invasive. I expect expanding to 4096 (or PATH_MAX)
> > would be sufficient in practice.
> 
> Perhaps it is in most cases.  Having certainty would be better.  We can
> leave unpack_trees() untouched and instead traverse the tree beforehand,
> in order to find the longest path.  Perhaps we can do something similar
> for init_db().

I wonder if it would be possible to just wrap open() in a transparent way.

Show 8 quoted lines
> > I'd also note that the current code isn't _remotely_ async safe and
> > nobody's complained. So I'm perfectly happy doing nothing, too. I care
> > a bit more about the tempfile.c interface because it's invoked a lot
> > more frequently.
> 
> I guess clones are not interrupted very often during checkout; same
> with worktree creation.  So perhaps nasty things could happen with a
> low probability, but nobody tried hard enough to hit that case?

Right, that's my guess. And "nasty" is quite likely to be "deadlocks on a malloc or stdio lock". Which is not the end of the world.

Show 12 quoted lines
> > No, I think that's how you'd have to do it. The implementation isn't all
> > that bad, but hitting every file-creation would be invasive. I'd
> > almost rather bail to exec-ing "rm -rf $dir", but we probably can't do
> > _that_ in a signal-handler either. :)
> 
> Well, fork(2), execve(2), and waitpid(2) are on the list, so actually
> you can.
> 
> mingw_spawnvpe(), which is used by start_command() on Windows,
> allocates memory, though.  Preparing environment and argument list
> in advance and just calling CreateProcess() in the signal handler
> might work.
Yeah, I imagine it's do-able with enough advance effort.

Given the lack of reports and the rapidly expanding complexity, I'm not planning on looking further into this for now.

-Peff
Previous: René ScharfeNext: Rene Scharfe
Message 13 of 75 in “plug strbuf memory leaks”
  1. 00/34 plug strbuf memory leaksRene Scharfe, Aug 30, 2017
  2. 01/34 am: release strbufs after use in detect_patch_format()Rene Scharfe, Aug 30, 2017
  3. Stefan BellerAug 31, 2017
  4. 02/34 am: release strbuf on error return in hg_patch_to_mail()Rene Scharfe, Aug 30, 2017
  5. 05/34 clean: release strbuf after use in remove_dirs()Rene Scharfe, Aug 30, 2017
  6. 06/34 clone: release strbuf after use in remove_junk()Rene Scharfe, Aug 30, 2017
  7. Junio C HamanoSep 6, 2017
  8. René ScharfeSep 10, 2017
  9. Jeff KingSep 10, 2017
  10. René ScharfeSep 10, 2017
  11. Jeff KingSep 10, 2017
  12. René ScharfeSep 11, 2017
  13. Jeff KingSep 13, 2017
  14. 07/34 commit: release strbuf on error return in commit_tree_extended()Rene Scharfe, Aug 30, 2017
  15. Stefan BellerAug 31, 2017
  16. 19/34 merge: release strbuf after use in write_merge_heads()Rene Scharfe, Aug 30, 2017
  17. 10/34 diff: release strbuf after use in diff_summary()Rene Scharfe, Aug 30, 2017
  18. Stefan BellerAug 31, 2017
  19. 14/34 help: release strbuf on error return in exec_man_man()Rene Scharfe, Aug 30, 2017
  20. 15/34 help: release strbuf on error return in exec_woman_emacs()Rene Scharfe, Aug 30, 2017
  21. 18/34 merge: release strbuf after use in save_state()Rene Scharfe, Aug 30, 2017
  22. 16/34 mailinfo: release strbuf after use in handle_from()Rene Scharfe, Aug 30, 2017
  23. 17/34 mailinfo: release strbuf on error return in handle_boundary()Rene Scharfe, Aug 30, 2017
  24. Martin ÅgrenAug 30, 2017
  25. René ScharfeAug 31, 2017
  26. Martin ÅgrenSep 5, 2017
  27. 13/34 help: release strbuf on error return in exec_man_konqueror()Rene Scharfe, Aug 30, 2017
  28. 09/34 convert: release strbuf on error return in filter_buffer_or_fd()Rene Scharfe, Aug 30, 2017
  29. 08/34 connect: release strbuf on error return in git_connect()Rene Scharfe, Aug 30, 2017
  30. Stefan BellerAug 31, 2017
  31. 03/34 am: release strbuf after use in safe_to_abort()Rene Scharfe, Aug 30, 2017
  32. 04/34 check-ref-format: release strbuf after use in check_ref_format_branch()Rene Scharfe, Aug 30, 2017
  33. 11/34 diff: release strbuf after use in show_rename_copy()Rene Scharfe, Aug 30, 2017
  34. 12/34 diff: release strbuf after use in show_stats()Rene Scharfe, Aug 30, 2017
  35. 20/34 notes: release strbuf after use in notes_copy_from_stdin()Rene Scharfe, Aug 30, 2017
  36. 02/34 am: release strbuf on error return in hg_patch_to_mail()Rene Scharfe, Aug 30, 2017
  37. 03/34 am: release strbuf after use in safe_to_abort()Rene Scharfe, Aug 30, 2017
  38. 09/34 convert: release strbuf on error return in filter_buffer_or_fd()Rene Scharfe, Aug 30, 2017
  39. 04/34 check-ref-format: release strbuf after use in check_ref_format_branch()Rene Scharfe, Aug 30, 2017
  40. 12/34 diff: release strbuf after use in show_stats()Rene Scharfe, Aug 30, 2017
  41. 11/34 diff: release strbuf after use in show_rename_copy()Rene Scharfe, Aug 30, 2017
  42. 21/34 refs: release strbuf on error return in write_pseudoref()Rene Scharfe, Aug 30, 2017
  43. 08/34 connect: release strbuf on error return in git_connect()Rene Scharfe, Aug 30, 2017
  44. 08/34 connect: release strbuf on error return in git_connect()Rene Scharfe, Aug 30, 2017
  45. 22/34 remote: release strbuf after use in read_remote_branches()Rene Scharfe, Aug 30, 2017
  46. 21/34 refs: release strbuf on error return in write_pseudoref()Rene Scharfe, Aug 30, 2017
  47. 25/34 send-pack: release strbuf on error return in send_pack()Rene Scharfe, Aug 30, 2017
  48. 23/34 remote: release strbuf after use in migrate_file()Rene Scharfe, Aug 30, 2017
  49. 24/34 remote: release strbuf after use in set_url()Rene Scharfe, Aug 30, 2017
  50. 27/34 shortlog: release strbuf after use in insert_one_record()Rene Scharfe, Aug 30, 2017
  51. Junio C HamanoSep 6, 2017
  52. Jeff KingSep 7, 2017
  53. Junio C HamanoSep 8, 2017
  54. Jeff KingSep 8, 2017
  55. Jeff KingSep 8, 2017
  56. Junio C HamanoSep 8, 2017
  57. shortlog: skip format/parse roundtrip for internal traversalJeff King, Sep 8, 2017
  58. René ScharfeSep 10, 2017
  59. Jeff KingSep 10, 2017
  60. 26/34 sha1_file: release strbuf on error return in index_path()Rene Scharfe, Aug 30, 2017
  61. 08/34 connect: release strbuf on error return in git_connect()Rene Scharfe, Aug 30, 2017
  62. 28/34 sequencer: release strbuf after use in save_head()Rene Scharfe, Aug 30, 2017
  63. 32/34 vcs-svn: release strbuf after use in end_revision()Rene Scharfe, Aug 30, 2017
  64. 34/34 wt-status: release strbuf after use in wt_longstatus_print_tracking()Rene Scharfe, Aug 30, 2017
  65. Junio C HamanoSep 6, 2017
  66. René ScharfeSep 10, 2017
  67. Junio C HamanoSep 10, 2017
  68. 31/34 utf8: release strbuf on error return in strbuf_utf8_replace()Rene Scharfe, Aug 30, 2017
  69. 33/34 wt-status: release strbuf after use in read_rebase_todolist()Rene Scharfe, Aug 30, 2017
  70. 21/34 refs: release strbuf on error return in write_pseudoref()Rene Scharfe, Aug 30, 2017
  71. 25/34 send-pack: release strbuf on error return in send_pack()Rene Scharfe, Aug 30, 2017
  72. 29/34 transport-helper: release strbuf after use in process_connect_service()Rene Scharfe, Aug 30, 2017
  73. 30/34 userdiff: release strbuf after use in userdiff_get_textconv()Rene Scharfe, Aug 30, 2017
  74. Stefan BellerAug 31, 2017
  75. Junio C HamanoSep 6, 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.