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

Re: [PATCH v2 18/25] lockfile: avoid transitory invalid states

From
Jeff King <peff@peff.net>
Date
Apr 7, 2014, 19:38 UTC
Message-ID
<20140407193834.GD19342@sigill.intra.peff.net>
In-Reply-To
<5342A451.5020905@alum.mit.edu>
On Mon, Apr 07, 2014 at 03:12:49PM +0200, Michael Haggerty wrote:
Show 10 quoted lines
> > How far *do* you want to go? I'm certainly not opposed to field-test your
> > current changeset (plus and adjustment to use sig_atomic_t) -- overall it
> > is an improvement. And then we will see how it works.
> 
> For now I think I'd just like to get the biggest problems fixed without
> making anything worse.  Given that there might be a GSoC student working
> in this neighborhood, he/she might be able to take up the baton.
> 
> I changed the patch series to use a new "volatile sig_atomic_t active"
> field rather than a bit in a "flags" field.
That seems like a good place to stop for now.

If any code touches the fields, it can unset the "active" flag (even temporarily), and restore it when the structure is in a known state.

I'm not sure if we can ever reach full safety. If you have an event loop, the sane thing to do is set an atomic flag in your signal handler and then handle it on the next iteration of the loop. But all of our signal handlers are jumped to from arbitrary code, and are just going to die(). There's nothing to return to, so any useful work we do has to be done from the handler.

-Peff
Previous: Michael HaggertyNext: Michael Haggerty
Message 29 of 37 in “Lockfile correctness and refactoring”
  1. 00/25 Lockfile correctness and refactoringMichael Haggerty, Apr 6, 2014
  2. 01/25 api-lockfile: expand the documentationMichael Haggerty, Apr 6, 2014
  3. Jeff KingApr 7, 2014
  4. 02/25 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Apr 6, 2014
  5. Jeff KingApr 7, 2014
  6. Michael HaggertyApr 8, 2014
  7. 03/25 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Apr 6, 2014
  8. 04/25 rollback_lock_file(): set fd to -1Michael Haggerty, Apr 6, 2014
  9. 05/25 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Apr 6, 2014
  10. 06/25 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Apr 6, 2014
  11. 07/25 lock_file(): always add lock_file object to lock_file_listMichael Haggerty, Apr 6, 2014
  12. 08/25 struct lock_file: replace on_list field with flags fieldMichael Haggerty, Apr 6, 2014
  13. 09/25 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Apr 6, 2014
  14. 10/25 lockfile: define a constant LOCK_SUFFIX_LENMichael Haggerty, Apr 6, 2014
  15. 11/25 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Apr 6, 2014
  16. 12/25 prepare_index(): declare return value to be (const char *)Michael Haggerty, Apr 6, 2014
  17. 13/25 write_packed_entry_fn(): convert cb_data into a (const int *)Michael Haggerty, Apr 6, 2014
  18. 14/25 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Apr 6, 2014
  19. 15/25 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Apr 6, 2014
  20. 16/25 commit_lock_file(): inline temporary variableMichael Haggerty, Apr 6, 2014
  21. 17/25 commit_lock_file(): make committing an unlocked lockfile a NOPMichael Haggerty, Apr 6, 2014
  22. Jeff KingApr 7, 2014
  23. Michael HaggertyApr 14, 2014
  24. 18/25 lockfile: avoid transitory invalid statesMichael Haggerty, Apr 6, 2014
  25. Johannes SixtApr 7, 2014
  26. Michael HaggertyApr 7, 2014
  27. Johannes SixtApr 7, 2014
  28. Michael HaggertyApr 7, 2014
  29. Jeff KingApr 7, 2014
  30. 19/25 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Apr 6, 2014
  31. 20/25 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Apr 6, 2014
  32. 21/25 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Apr 6, 2014
  33. 22/25 Change lock_file::filename into a strbufMichael Haggerty, Apr 6, 2014
  34. 23/25 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Apr 6, 2014
  35. 24/25 resolve_symlink(): take a strbuf parameterMichael Haggerty, Apr 6, 2014
  36. 25/25 trim_last_path_elm(): replace last_path_elm()Michael Haggerty, Apr 6, 2014
  37. Jeff KingApr 7, 2014

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.