Interesting. SecItemUpdate returning errSecDuplicateItem didn’t make sense to make sense to me so I had a check to see what scenario this happens and it appears to be a scenario where updating in-place fails but replacing it entirely succeeds. However it seems the item might have ultimately still been updated: https://github.com/apple-oss-distributions/Security/blob/0600e7bab30fbac3adcafcb6c57d3981dc682304/OSX/libsecurity_keychain/lib/SecItem.cpp#L2398 The behaviour is a bit odd and the associated code comment referencing an Apple bug number is perhaps is indicative of that. I guess it perhaps makes sense if you are holding references, but that doesn’t apply to us. I wonder if a fix here could be to treat errSecDuplicateItem as a successful operation for SecItemUpdate. Can you confirm the keychain item is successfully updated in that scenario? A broader Git-wide question that you perhaps don’t know the answer to but someone else here might do is: why are we spamming updates to the credential helper? Every parallel fetch instance performing a store operation on the same host seems unexpected to me, particularly if there’s no actual changes. Bo > On 10 May 2024, at 09:07, Koji Nakamaru via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Koji Nakamaru <koji.nakamaru@xxxxxxxx> > > Resolves "failed to store: -25299" when "fetch.parallel 0" is configured > and there are many submodules. > > The error code -25299 (errSecDuplicateItem) may be returned by > SecItemUpdate() in add_internet_password() if multiple instances of > git-credential-osxkeychain run in parallel. This patch introduces an > exclusive lock to serialize execution for avoiding this and other > potential issues. > > Signed-off-by: Koji Nakamaru <koji.nakamaru@xxxxxxxx> > --- > osxkeychain: lock for exclusive execution > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1729%2FKojiNakamaru%2Ffeature%2Fosxkeychian_exlock-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1729/KojiNakamaru/feature/osxkeychian_exlock-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1729 > > contrib/credential/osxkeychain/git-credential-osxkeychain.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c > index 6a40917b1ef..0884db48d0a 100644 > --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c > +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c > @@ -414,6 +414,9 @@ int main(int argc, const char **argv) > if (!argv[1]) > die("%s", usage); > > + if (open(argv[0], O_RDONLY | O_EXLOCK) == -1) > + die("failed to lock %s", argv[0]); > + > read_credential(); > > if (!strcmp(argv[1], "get")) > > base-commit: 0f3415f1f8478b05e64db11eb8aaa2915e48fef6 > -- > gitgitgadget
