{"thread":{"id":"62914","subject":"Unknown error with concurrent config read/write on Windows","startedAt":"2025-02-07T02:44:44Z","lastAt":"2025-03-19T22:16:57Z","messageCount":3,"participants":["Allen Li","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"512023","messageId":"CADbSrJzNCOP0t=Vsdopa2+GFth_J84E8SEvpRJYfg8uxYnH3ng@mail.gmail.com","threadId":"62914","inReplyTo":null,"subject":"Unknown error with concurrent config read/write on Windows","fromName":"Allen Li","fromEmail":"darkfeline@felesatra.moe","sentAt":"2025-02-07T02:44:33Z","receivedAt":"2025-02-07T02:44:44Z","isPatch":false,"sender":{"key":"darkfeline@felesatra.moe","avatar":null},"body":"On Windows, Git encounters fatal errors (that do not happen on Linux)\nwhen reading and writing the Git config in parallel processes.\n\nI and some peers can relatively easily reproduce this by running in\nparallel (e.g., in two separate terminal/PowerShell windows):\n\nfor (;;) {git rev-parse HEAD}\nfor (;;) {git config --local user.name foo}\n\nThe rev-parse \"thread\" will spew messages like:\n\nf100762176b7b085e81cafe261a049d809772ace\nf100762176b7b085e81cafe261a049d809772ace\nf100762176b7b085e81cafe261a049d809772ace\nwarning: unable to access '.git/config': Permission denied\nfatal: unknown error occurred while reading the configuration files\nf100762176b7b085e81cafe261a049d809772ace\n\nThis affects any command that reads the Git config (which is all/most\nof them); rev-parse is just a convenient stand-in.\n\nI have seen the work to make lockfile renames (and thus config edits I\nbelieve) more atomic in\nhttps://lore.kernel.org/all/cover.1729695349.git.ps@pks.im/T/ which\nshould be in 28.0 IIUC.\n\nHowever, the above issue can still be reproduced with an RC build of\n28.0 (of the Git for Windows distribution).\n\nBased on where the unknown error text occurs in the code, I speculate\nthat the \"atomic rename\" of config.lock to config is allowing\nsubsequent readers to read an incomplete version of the config file,\nwhich means that the fix I referenced above may be deficient in some\nway.\n\nI will omit the context around my problem, which can be summarized as\n\"legacy infrastructure/scripts\", but I'd like to frame this bug report\nas \"this works on Linux, but not on Windows\" in the hopes of getting\nsome help, although I understand this may not be the highest priority.\n\n(Apologies in advance for my ignorance with Windows and/or using this\nmailing list.)\n"},{"id":"512892","messageId":"Z7xrr-O1QdhXWftj@pks.im","threadId":"62914","inReplyTo":"CADbSrJzNCOP0t=Vsdopa2+GFth_J84E8SEvpRJYfg8uxYnH3ng@mail.gmail.com","subject":"Re: Unknown error with concurrent config read/write on Windows","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-24T12:53:03Z","receivedAt":"2025-02-24T12:53:14Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 06, 2025 at 06:44:33PM -0800, Allen Li wrote:\n> On Windows, Git encounters fatal errors (that do not happen on Linux)\n> when reading and writing the Git config in parallel processes.\n> \n> I and some peers can relatively easily reproduce this by running in\n> parallel (e.g., in two separate terminal/PowerShell windows):\n> \n> for (;;) {git rev-parse HEAD}\n> for (;;) {git config --local user.name foo}\n> \n> The rev-parse \"thread\" will spew messages like:\n> \n> f100762176b7b085e81cafe261a049d809772ace\n> f100762176b7b085e81cafe261a049d809772ace\n> f100762176b7b085e81cafe261a049d809772ace\n> warning: unable to access '.git/config': Permission denied\n> fatal: unknown error occurred while reading the configuration files\n> f100762176b7b085e81cafe261a049d809772ace\n> \n> This affects any command that reads the Git config (which is all/most\n> of them); rev-parse is just a convenient stand-in.\n> \n> I have seen the work to make lockfile renames (and thus config edits I\n> believe) more atomic in\n> https://lore.kernel.org/all/cover.1729695349.git.ps@pks.im/T/ which\n> should be in 28.0 IIUC.\n> \n> However, the above issue can still be reproduced with an RC build of\n> 28.0 (of the Git for Windows distribution).\n\nI'm not quite sure which version you're referring to here. 28.0 is not a\nversion released by either GIt nor Git for Windows. You probably meant\nto refer to v2.48.0?\n\nIn any case, it's not entirely surprising that this may still cause\nissues in some code paths. Support for POSIX-style renames requires two\ndifferent bits:\n\n  - The `rename()` implementation needs to know to allow POSIX-style\n    renames even when the target file is currently held open.\n\n  - All code paths that open a file need to be taught to open them with\n    `FILE_SHARE_DELETE`. This flag allows the file to be deleted while\n    the file handle remains open.\n\nThe first part has been implemented by the mentioned patch series, and\nsome code paths have been adapted to also do the second part. But not\nall code paths do this yet, and those that don't will not be able to use\natomic renames when the file is open.\n\nOne important omission in your context is that fopen(3p) does not yet\nknow to set `FILE_SHARE_DELETE`. It uses `_wfopen()` right now, which\ndoes not set this flag. In the best case we'd convert the code to use\n`_wfsopen()` instead, which allows us to control the sharing mode. But\nunfortunately, it only allows us to control `FILe_SHARE_WRITE` as well\nas `FILE_SHARE_READ`, but not `FILE_SHARE_DELETE`.\n\nSo to the best of my knowledge, we'd have to reimplement the function on\ntop of `CreateFileW()` so that we can fully control the file sharing\nmode.\n\nPatrick\n"},{"id":"514681","messageId":"CADbSrJxDP6ZFqE709ZBeg76LUsXkxwWeQBCXRuY9oPO8fiAAMQ@mail.gmail.com","threadId":"62914","inReplyTo":"Z7xrr-O1QdhXWftj@pks.im","subject":"Re: Unknown error with concurrent config read/write on Windows","fromName":"Allen Li","fromEmail":"darkfeline@felesatra.moe","sentAt":"2025-03-19T22:16:45Z","receivedAt":"2025-03-19T22:16:57Z","isPatch":false,"sender":{"key":"darkfeline@felesatra.moe","avatar":null},"body":"On Mon, Feb 24, 2025 at 4:53 AM Patrick Steinhardt <ps@pks.im> wrote:\n> I'm not quite sure which version you're referring to here. 28.0 is not a\n> version released by either GIt nor Git for Windows. You probably meant\n> to refer to v2.48.0?\n\nYes sorry, I meant v2.48.0\n\n> In any case, it's not entirely surprising that this may still cause\n> issues in some code paths. Support for POSIX-style renames requires two\n> different bits:\n>\n>   - The `rename()` implementation needs to know to allow POSIX-style\n>     renames even when the target file is currently held open.\n>\n>   - All code paths that open a file need to be taught to open them with\n>     `FILE_SHARE_DELETE`. This flag allows the file to be deleted while\n>     the file handle remains open.\n>\n> The first part has been implemented by the mentioned patch series, and\n> some code paths have been adapted to also do the second part. But not\n> all code paths do this yet, and those that don't will not be able to use\n> atomic renames when the file is open.\n>\n> One important omission in your context is that fopen(3p) does not yet\n> know to set `FILE_SHARE_DELETE`. It uses `_wfopen()` right now, which\n> does not set this flag. In the best case we'd convert the code to use\n> `_wfsopen()` instead, which allows us to control the sharing mode. But\n> unfortunately, it only allows us to control `FILe_SHARE_WRITE` as well\n> as `FILE_SHARE_READ`, but not `FILE_SHARE_DELETE`.\n>\n> So to the best of my knowledge, we'd have to reimplement the function on\n> top of `CreateFileW()` so that we can fully control the file sharing\n> mode.\n\nThank you, that makes sense.  We ended up committing to a different\napproach which\nsidesteps this problem, but I think there's enough context here for\nanyone else who\nmay experience this problem to take a shot at fixing it (perhaps it\nwill be future me).\n\n> Patrick\n"}]}