Re: [PATCH v2] config: retry acquiring config.lock, configurable via core.configLockTimeout

From: Junio C Hamano <gitster@pobox.com>
Date: May 18, 2026, 00:46 UTC
Message-ID: <xmqqzf1xbl4i.fsf@gitster.g>
In-Reply-To: <20260517132111.1014901-1-joerg@thalheim.io>

Joerg Thalheim <joerg@thalheim.io> writes:

Show 17 quoted lines

> +/*
> + * How long to retry acquiring config.lock when another process holds
> + * it. Default matches core.packedRefsTimeout; override via
> + * core.configLockTimeout.
> + */
> +static long config_lock_timeout_ms(struct repository *r)
> +{
> +	static int configured;
> +	static int timeout_ms = 1000;
> +
> +	if (!configured) {
> +		repo_config_get_int(r, "core.configlocktimeout", &timeout_ms);
> +		configured = 1;
> +	}
> +
> +	return timeout_ms;
> +}

The above design means whichever repository happens to be passed for the first time as "r" to this call will fix the return value from the function for the rest of the system, meaning that the lock timeout is a per-process property and the repository parameter passed to the function does not matter all that much.

It may make sense to admit that this is not a per-repository property (due to the use of local caching), have the function take no parameter and use the_repository to the config_get call. That would make the intention more clear.

Of course the other end of the spectrum is to get rid of the "configured" caching here, and ask the config system to make a hashtable look-up every time the function is called. That will keep the lock timeout per-repository, which is closer to what the current function signature suggests.

I dunno. My gut feeling is that there aren't valid reasons why you would want to specifically set different timeout values per repository, so the simplicity of using the_repository (i.e. the primary repository instance this process deals with) sounds like a better way to go.

Thanks.