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
Show 42 quoted lines
> On 10 May 2024, at 09:07, Koji Nakamaru via GitGitGadget <gitgitgadget@gmail.com> wrote:
>
> From: Koji Nakamaru <koji.nakamaru@gree.net>
>
> 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@gree.net>
> ---
> 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
> --
> gitgitgadgetBo Anderson <mail@boanderson.me>