Koji Nakamaru (2): osxkeychain: exclusive lock to serialize execution of operations osxkeychain: state to skip unnecessary store operations .../osxkeychain/git-credential-osxkeychain.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) base-commit: 83f1add914c6b4682de1e944ec0d1ac043d53d78 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1729%2FKojiNakamaru%2Ffeature%2Fosxkeychian_exlock-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1729/KojiNakamaru/feature/osxkeychian_exlock-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1729 Range-diff vs v2: 1: 309c17c78f3 ! 1: 3341346c5e6 osxkeychain: lock for exclusive execution @@ Metadata Author: Koji Nakamaru <koji.nakamaru@xxxxxxxx> ## Commit message ## - osxkeychain: lock for exclusive execution + osxkeychain: exclusive lock to serialize execution of operations - Resolves "failed to store: -25299" when "fetch.parallel 0" is configured - and there are many submodules. + git passes a credential that has been used successfully to the helpers + to record. If "git-credential-osxkeychain store" commands run in + parallel (with fetch.parallel configuration and/or by running multiple + git commands simultaneously), some of them may exit with the error + "failed to store: -25299". This is because SecItemUpdate() in + add_internet_password() may return errSecDuplicateItem (-25299) in this + situation. Apple's documentation [1] also states as below: - 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. + In macOS, some of the functions of this API block while waiting for + input from the user (for example, when the user is asked to unlock a + keychain or give permission to change trust settings). In general, it + is safe to use this API in threads other than your main thread, but + avoid calling the functions from multiple operations, work queues, or + threads concurrently. Instead, serialize function calls or confine + them to a single thread. + + The error has not been noticed before, because the former implementation + ignored the error. + + Introduce an exclusive lock to serialize execution of operations. + + [1] https://developer.apple.com/documentation/security/certificate_key_and_trust_services/working_with_concurrency Signed-off-by: Koji Nakamaru <koji.nakamaru@xxxxxxxx> 2: 1f57718abff ! 2: 146b0ae9146 osxkeychain: state[] seen=1 to skip unnecessary store operations @@ Metadata Author: Koji Nakamaru <koji.nakamaru@xxxxxxxx> ## Commit message ## - osxkeychain: state[] seen=1 to skip unnecessary store operations + osxkeychain: state to skip unnecessary store operations - Records whether credentials come from get operations and skips - unnecessary store operations by utilizing the state[] feature, as - suggested by brian m. carlson. + git passes a credential that has been used successfully to the helpers + to record. If a credential is already stored, + "git-credential-osxkeychain store" just records the credential returned + by "git-credential-osxkeychain get", and unnecessary (sometimes + problematic) SecItemAdd() and/or SecItemUpdate() are performed. + We can skip such unnecessary operations by marking a credential returned + by "git-credential-osxkeychain get". This marking can be done by + utilizing the "state[]" feature: + + - The "get" command sets the field "state[]=osxkeychain:seen=1". + + - The "store" command skips its actual operation if the field + "state[]=osxkeychain:seen=1" exists. + + Introduce a new state "state[]=osxkeychain:seen=1". + + Suggested-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Koji Nakamaru <koji.nakamaru@xxxxxxxx> ## contrib/credential/osxkeychain/git-credential-osxkeychain.c ## -- gitgitgadget
