From: Kyle <kiltnaked@gmail.com>
Date: Fri, 03 Jan 2014 00:11:24 GMT
Subject: Re: [PATCH] gc: notice gc processes run by other users
Message-ID: <02BCE0EB-ADA1-48F9-BD00-369FFDB5E372@gmail.com>
In-Reply-To: <720d7a5676f8cbfc76c80198f9d3816@74d39fa044aa309eaea14b9f57fe79c>

On Dec 31, 2013, at 04:07, Kyle J. McKay wrote:
> Since 64a99eb4 git gc refuses to run without the --force option if
> another gc process on the same repository is already running.
>
> However, if the repository is shared and user A runs git gc on the
> repository and while that gc is still running user B runs git gc on
> the same repository the gc process run by user A will not be noticed
> and the gc run by user B will go ahead and run.
>
> The problem is that the kill(pid, 0) test fails with an EPERM error
> since user B is not allowed to signal processes owned by user A
> (unless user B is root).
>
> Update the test to recognize an EPERM error as meaning the process
> exists and another gc should not be run (unless --force is given).

Oops, sorry, forgot sign off, here's my sign off:

Signed-off-by: Kyle J. McKay <mackyle@gmail.com>

> ---
>
> I suggest this be included in maint as others may also have expected  
> the
> shared repository, different user gc scenario to be caught by the new
> code when in fact it's not without this patch.
>
> builtin/gc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index c14190f8..25f2237c 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -222,7 +222,7 @@ static const char *lock_repo_for_gc(int force,  
> pid_t* ret_pid)
> 			time(NULL) - st.st_mtime <= 12 * 3600 &&
> 			fscanf(fp, "%"PRIuMAX" %127c", &pid, locking_host) == 2 &&
> 			/* be gentle to concurrent "gc" on remote hosts */
> -			(strcmp(locking_host, my_host) || !kill(pid, 0));
> +			(strcmp(locking_host, my_host) || !kill(pid, 0) || errno ==  
> EPERM);
> 		if (fp != NULL)
> 			fclose(fp);
> 		if (should_exit) {
> -- 
> 1.8.5.2
>