{"thread":{"id":"16789","subject":"git-rm -n leaves .git/index.lock if not allowed to finish","startedAt":"2008-12-18T20:02:48Z","lastAt":"2008-12-19T03:47:57Z","messageCount":6,"participants":["jidanni@jidanni.org","Miklos Vajna","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"98294","messageId":"87prjptfo7.fsf@jidanni.org","threadId":"16789","inReplyTo":null,"subject":"git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"","fromEmail":"jidanni@jidanni.org","sentAt":"2008-12-18T20:02:48Z","receivedAt":"2008-12-18T20:02:48Z","isPatch":false,"sender":{"key":"jidanni@jidanni.org","avatar":"https://gravatar.com/avatar/36568d4af4c8d3e71627ef3b8c8d00e39065b12f29676cccd38ced75e68fa2a6?d=mp&s=160"},"body":"Bug: if git-rm -n is not allowed to write all it wants to write, it\nwill leave a .git/index.lock file:\n# git-rm -n -r . 2>&1|sed q\nerror: '.etckeeper' has changes staged in the index\n# git-rm -n -r . 2>&1|sed q\nfatal: unable to create '.git/index.lock': File exists\n"},{"id":"98308","messageId":"20081219002524.GB21154@genesis.frugalware.org","threadId":"16789","inReplyTo":"87prjptfo7.fsf@jidanni.org","subject":"Re: git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-19T00:25:24Z","receivedAt":"2008-12-19T00:25:24Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Fri, Dec 19, 2008 at 04:02:48AM +0800, jidanni@jidanni.org wrote:\n> Bug: if git-rm -n is not allowed to write all it wants to write, it\n> will leave a .git/index.lock file:\n> # git-rm -n -r . 2>&1|sed q\n> error: '.etckeeper' has changes staged in the index\n> # git-rm -n -r . 2>&1|sed q\n> fatal: unable to create '.git/index.lock': File exists\n\nCan't reproduce:\n\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex b7d46e5..1581691 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -251,4 +251,12 @@ test_expect_success 'refresh index before checking if it is up-to-date' '\n \n '\n \n+test_expect_success 'test from jidanni' '\n+\n+\tgit reset --hard &&\n+\tgit rm -n -r . 2>&1|sed q &&\n+\tgit rm -n -r . 2>&1|sed q\n+\n+'\n+\n test_done\n\npasses here just fine. Yes, there are multiple files in the repo.\n"},{"id":"98311","messageId":"7v63lhf1cl.fsf@gitster.siamese.dyndns.org","threadId":"16789","inReplyTo":"20081219002524.GB21154@genesis.frugalware.org","subject":"Re: git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-19T00:35:54Z","receivedAt":"2008-12-19T00:35:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> On Fri, Dec 19, 2008 at 04:02:48AM +0800, jidanni@jidanni.org wrote:\n>> Bug: if git-rm -n is not allowed to write all it wants to write, it\n>> will leave a .git/index.lock file:\n>> # git-rm -n -r . 2>&1|sed q\n>> error: '.etckeeper' has changes staged in the index\n>> # git-rm -n -r . 2>&1|sed q\n>> fatal: unable to create '.git/index.lock': File exists\n>\n> Can't reproduce:\n>\n> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\n> index b7d46e5..1581691 100755\n> --- a/t/t3600-rm.sh\n> +++ b/t/t3600-rm.sh\n> @@ -251,4 +251,12 @@ test_expect_success 'refresh index before checking if it is up-to-date' '\n>  \n>  '\n>  \n> +test_expect_success 'test from jidanni' '\n> +\n> +\tgit reset --hard &&\n> +\tgit rm -n -r . 2>&1|sed q &&\n> +\tgit rm -n -r . 2>&1|sed q\n> +\n> +'\n> +\n>  test_done\n>\n> passes here just fine. Yes, there are multiple files in the repo.\n\nI think you need to have tons of files to cause the pipe buffer to fill up\nwith the \"rm 'frotz'\" output, triggering a SIGPIPE to kill the upstream\nprocess of the pipe.\n"},{"id":"98312","messageId":"7v4p11f18d.fsf@gitster.siamese.dyndns.org","threadId":"16789","inReplyTo":"20081219002524.GB21154@genesis.frugalware.org","subject":"Re: git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-19T00:38:26Z","receivedAt":"2008-12-19T00:38:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> On Fri, Dec 19, 2008 at 04:02:48AM +0800, jidanni@jidanni.org wrote:\n>> Bug: if git-rm -n is not allowed to write all it wants to write, it\n>> will leave a .git/index.lock file:\n>> # git-rm -n -r . 2>&1|sed q\n>> error: '.etckeeper' has changes staged in the index\n>> # git-rm -n -r . 2>&1|sed q\n>> fatal: unable to create '.git/index.lock': File exists\n>\n> Can't reproduce:\n>\n> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\n> index b7d46e5..1581691 100755\n> --- a/t/t3600-rm.sh\n> +++ b/t/t3600-rm.sh\n> @@ -251,4 +251,12 @@ test_expect_success 'refresh index before checking if it is up-to-date' '\n>  \n>  '\n>  \n> +test_expect_success 'test from jidanni' '\n> +\n> +\tgit reset --hard &&\n> +\tgit rm -n -r . 2>&1|sed q &&\n> +\tgit rm -n -r . 2>&1|sed q\n> +\n> +'\n> +\n>  test_done\n>\n> passes here just fine. Yes, there are multiple files in the repo.\n\nI think you need to have tons of files to cause the pipe buffer to fill up\nwith the \"rm 'frotz'\" output, triggering a SIGPIPE to kill the upstream\nprocess of the pipe.\n\nWould this patch help?\n\n lockfile.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git i/lockfile.c w/lockfile.c\nindex 6d75608..8589155 100644\n--- i/lockfile.c\n+++ w/lockfile.c\n@@ -140,6 +140,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \t\t\tsignal(SIGHUP, remove_lock_file_on_signal);\n \t\t\tsignal(SIGTERM, remove_lock_file_on_signal);\n \t\t\tsignal(SIGQUIT, remove_lock_file_on_signal);\n+\t\t\tsignal(SIGPIPE, remove_lock_file_on_signal);\n \t\t\tatexit(remove_lock_file);\n \t\t}\n \t\tlk->owner = getpid();\n"},{"id":"98318","messageId":"7vljuddk6q.fsf@gitster.siamese.dyndns.org","threadId":"16789","inReplyTo":"7v4p11f18d.fsf@gitster.siamese.dyndns.org","subject":"Re: git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-19T01:31:57Z","receivedAt":"2008-12-19T01:31:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Subject: [PATCH] Make sure lockfiles are unlocked when dying on SIGPIPE\n\nWe cleaned up lockfiles upon receiving the usual suspects HUP, TERM, QUIT\nbut a wicked user could kill us of asphyxiation by piping our output to a\npipe that does not read.  Protect ourselves by catching SIGPIPE and clean\nup the lockfiles as well in such a case.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n lockfile.c    |    1 +\n t/t3600-rm.sh |   17 +++++++++++++++++\n 2 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git c/lockfile.c w/lockfile.c\nindex 6d75608..8589155 100644\n--- c/lockfile.c\n+++ w/lockfile.c\n@@ -140,6 +140,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \t\t\tsignal(SIGHUP, remove_lock_file_on_signal);\n \t\t\tsignal(SIGTERM, remove_lock_file_on_signal);\n \t\t\tsignal(SIGQUIT, remove_lock_file_on_signal);\n+\t\t\tsignal(SIGPIPE, remove_lock_file_on_signal);\n \t\t\tatexit(remove_lock_file);\n \t\t}\n \t\tlk->owner = getpid();\ndiff --git c/t/t3600-rm.sh w/t/t3600-rm.sh\nindex b7d46e5..95542e9 100755\n--- c/t/t3600-rm.sh\n+++ w/t/t3600-rm.sh\n@@ -251,4 +251,21 @@ test_expect_success 'refresh index before checking if it is up-to-date' '\n \n '\n \n+test_expect_success 'choking \"git rm\" should not let it die with cruft' '\n+\tgit reset -q --hard &&\n+\tH=0000000000000000000000000000000000000000 &&\n+\ti=0 &&\n+\twhile test $i -lt 12000\n+\tdo\n+\t    echo \"100644 $H 0\tsome-file-$i\"\n+\t    i=$(( $i + 1 ))\n+\tdone | git update-index --index-info &&\n+\tgit rm -n \"some-file-*\" | :;\n+\ttest -f .git/index.lock\n+\tstatus=$?\n+\trm -f .git/index.lock\n+\tgit reset -q --hard\n+\ttest \"$status\" != 0\n+'\n+\n test_done\n"},{"id":"98330","messageId":"20081219034757.GC21154@genesis.frugalware.org","threadId":"16789","inReplyTo":"7v63lhf1cl.fsf@gitster.siamese.dyndns.org","subject":"Re: git-rm -n leaves .git/index.lock if not allowed to finish","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-19T03:47:57Z","receivedAt":"2008-12-19T03:47:57Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Dec 18, 2008 at 04:35:54PM -0800, Junio C Hamano <gitster@pobox.com> wrote:\n> I think you need to have tons of files to cause the pipe buffer to fill up\n> with the \"rm 'frotz'\" output, triggering a SIGPIPE to kill the upstream\n> process of the pipe.\n\nAh, you are right. I did exactly what is in the lockfile.c part of your\npatch, but I did not think about \"if I don't get a SIGPIPE for 2 lines,\nI may get one for 1000 lines\". ;-)\n"}]}