{"thread":{"id":"30319","subject":"[PATCH 1/3] git-svn: use POSIX::sigprocmask to block signals","startedAt":"2012-04-02T13:29:32Z","lastAt":"2012-04-24T19:09:47Z","messageCount":6,"participants":["Roman Kagan","Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"189934","messageId":"35ffdcb9d4a61885debd0fca9020c9eda98534b4.1335250396.git.rkagan@mail.ru","threadId":"30319","inReplyTo":"cover.1335250396.git.rkagan@mail.ru","subject":"[PATCH 1/3] git-svn: use POSIX::sigprocmask to block signals","fromName":"Roman Kagan","fromEmail":"rkagan@mail.ru","sentAt":"2012-04-02T13:29:32Z","receivedAt":"2012-04-02T13:29:32Z","isPatch":true,"sender":{"key":"rkagan@mail.ru","avatar":"https://avatars.githubusercontent.com/u/10214432?v=4"},"body":"In order to maintain consistency of the database mapping svn revision\nnumbers to git commit ids, rev_map_set() defers signal processing until\nit's finished with an append transaction.[*]\n\nThe conventional way to achieve this is through sigprocmask(), which is\navailable in perl in the standard POSIX module.\n\nThis is implemented by this patch. One important consequence of it is\nthat the signal handlers won't be unconditionally set to SIG_DFL anymore\nupon the first invocation of rev_map_set() as they used to. As a\nresult, the signals ignored by git-svn parent will remain ignored;\notherwise the behavior remains the same.\n\nThis patch paves the way to ignoring SIGPIPE throughout git-svn which\nwill be done in the followup patch.\n\n[*] Deferring signals is not enough to ensure the database consistency:\nthe program may die on SIGKILL or power loss, run out of disk space,\netc. However that's a separate issue that this patch doesn't address.\n\nSigned-off-by: Roman Kagan \n---\n git-svn.perl | 15 +++++++++------\n 1 files changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4334b95..570504c 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2031,6 +2031,7 @@ use IPC::Open3;\n use Time::Local;\n use Memoize; # core since 5.8.0, Jul 2002\n use Memoize::Storable;\n+use POSIX qw(:signal_h);\n \n my ($_gc_nr, $_gc_period);\n \n@@ -4059,11 +4060,14 @@ sub rev_map_set {\n \tlength $commit == 40 or die \"arg3 must be a full SHA1 hexsum\\n\";\n \tmy $db = $self->map_path($uuid);\n \tmy $db_lock = \"$db.lock\";\n-\tmy $sig;\n+\tmy $sigmask;\n \t$update_ref ||= 0;\n \tif ($update_ref) {\n-\t\t$SIG{INT} = $SIG{HUP} = $SIG{TERM} = $SIG{ALRM} = $SIG{PIPE} =\n-\t\t $SIG{USR1} = $SIG{USR2} = sub { $sig = $_[0] };\n+\t\t$sigmask = POSIX::SigSet->new();\n+\t\tmy $signew = POSIX::SigSet->new(SIGINT, SIGHUP, SIGTERM,\n+\t\t\tSIGALRM, SIGPIPE, SIGUSR1, SIGUSR2);\n+\t\tsigprocmask(SIG_BLOCK, $signew, $sigmask) or\n+\t\t\tcroak \"Can't block signals: $!\";\n \t}\n \tmkfile($db);\n \n@@ -4102,9 +4106,8 @@ sub rev_map_set {\n \t \"$db_lock => $db ($!)\\n\";\n \tdelete $LOCKFILES{$db_lock};\n \tif ($update_ref) {\n-\t\t$SIG{INT} = $SIG{HUP} = $SIG{TERM} = $SIG{ALRM} = $SIG{PIPE} =\n-\t\t $SIG{USR1} = $SIG{USR2} = 'DEFAULT';\n-\t\tkill $sig, $$ if defined $sig;\n+\t\tsigprocmask(SIG_SETMASK, $sigmask) or\n+\t\t\tcroak \"Can't restore signal mask: $!\";\n \t}\n }\n \n-- \n1.7.7.6\n"},{"id":"189936","messageId":"eee8a701e14491463ee211e19958d5e6d4377b03.1335250396.git.rkagan@mail.ru","threadId":"30319","inReplyTo":"cover.1335250396.git.rkagan@mail.ru","subject":"[PATCH 2/3] git-svn: ignore SIGPIPE","fromName":"Roman Kagan","fromEmail":"rkagan@mail.ru","sentAt":"2012-04-02T13:52:34Z","receivedAt":"2012-04-02T13:52:34Z","isPatch":true,"sender":{"key":"rkagan@mail.ru","avatar":"https://avatars.githubusercontent.com/u/10214432?v=4"},"body":"In HTTP with keep-alive it's not uncommon for the client to notice that\nthe server decided to stop maintaining the current connection only when\nsending a new request. This naturally results in -EPIPE and possibly\nSIGPIPE.\n\nThe subversion library itself makes no provision for SIGPIPE. Some\ncombinations of the underlying libraries do (typically SIG_IGN-ing it),\nsome don't.\n\nPresumably for that reason all subversion commands set SIGPIPE to\nSIG_IGN early in their main()-s.\n\nSo should we.\n\nThis, together with the previous patch, fixes the notorious \"git-svn\ndied of signal 13\" problem (see e.g.\nhttp://thread.gmane.org/gmane.comp.version-control.git/134936).\n\nSigned-off-by: Roman Kagan \n---\n git-svn.perl | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 570504c..aa14564 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -36,6 +36,11 @@ $ENV{TZ} = 'UTC';\n $| = 1; # unbuffer STDOUT\n \n sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n+\n+# All SVN commands do it. Otherwise we may die on SIGPIPE when the remote\n+# repository decides to close the connection which we expect to be kept alive.\n+$SIG{PIPE} = 'IGNORE';\n+\n sub _req_svn {\n \trequire SVN::Core; # use()-ing this causes segfaults for me... *shrug*\n \trequire SVN::Ra;\n-- \n1.7.7.6\n"},{"id":"189935","messageId":"4ffb0267b9afed911665265524247a15f4e04c39.1335250396.git.rkagan@mail.ru","threadId":"30319","inReplyTo":"cover.1335250396.git.rkagan@mail.ru","subject":"[PATCH 3/3] git-svn: drop redundant blocking of SIGPIPE","fromName":"Roman Kagan","fromEmail":"rkagan@mail.ru","sentAt":"2012-04-23T16:26:56Z","receivedAt":"2012-04-23T16:26:56Z","isPatch":true,"sender":{"key":"rkagan@mail.ru","avatar":"https://avatars.githubusercontent.com/u/10214432?v=4"},"body":"Now that SIGPIPE is ignored there's no point blocking it.\n\nSigned-off-by: Roman Kagan \n---\n git-svn.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex aa14564..f8e9ef0 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4070,7 +4070,7 @@ sub rev_map_set {\n \tif ($update_ref) {\n \t\t$sigmask = POSIX::SigSet->new();\n \t\tmy $signew = POSIX::SigSet->new(SIGINT, SIGHUP, SIGTERM,\n-\t\t\tSIGALRM, SIGPIPE, SIGUSR1, SIGUSR2);\n+\t\t\tSIGALRM, SIGUSR1, SIGUSR2);\n \t\tsigprocmask(SIG_BLOCK, $signew, $sigmask) or\n \t\t\tcroak \"Can't block signals: $!\";\n \t}\n-- \n1.7.7.6\n"},{"id":"189937","messageId":"cover.1335250396.git.rkagan@mail.ru","threadId":"30319","inReplyTo":null,"subject":"[PATCH 0/3] git-svn: fixes for intermittent SIGPIPE","fromName":"Roman Kagan","fromEmail":"rkagan@mail.ru","sentAt":"2012-04-24T06:53:16Z","receivedAt":"2012-04-24T06:53:16Z","isPatch":true,"sender":{"key":"rkagan@mail.ru","avatar":"https://avatars.githubusercontent.com/u/10214432?v=4"},"body":"In my work environment subversion is still being used as the main\nrevision control system. Therefore many people who prefer to work with\ngit have to resort to git-svn.\n\nHowever, in many configurations it used to suffer from the notorious\n\"git-svn died of signal 13\" problem (see e.g.\nhttp://thread.gmane.org/gmane.comp.version-control.git/134936 and the\nlinks therein).\n\nI believe to have tracked down the issue to the connection being closed\nby the server when http keep-alive is in use, and the client dying on\nSIGPIPE because its handler is left at SIG_DFL when a new request is\nbeing made.\n\nThe patches have been tested on\n\n- Linux Fedora 16 x86_64, git 1.7.7.6, perl v5.14.2, svn 1.6.17\n- Windows 7 x64 + Cygwin, git 1.7.9, perl v5.10.1, svn 1.7.4,\n- Windows 7 x64 + MsysGit, git 1.7.9.msysgit.0, perl v5.8.8, svn 1.4.6\n\nThis is the third version of the series; compared to the previous submission\nthe log message of the first patch was reworded to make it more palatable.\n\nRoman Kagan (3):\n git-svn: use POSIX::sigprocmask to block signals\n git-svn: ignore SIGPIPE\n git-svn: drop redundant blocking of SIGPIPE\n\n git-svn.perl | 20 ++++++++++++++------\n 1 files changed, 14 insertions(+), 6 deletions(-)\n\n-- \n1.7.7.6\n"},{"id":"189963","messageId":"20120424094541.GA30803@dcvr.yhbt.net","threadId":"30319","inReplyTo":"cover.1335250396.git.rkagan@mail.ru","subject":"Re: [PATCH 0/3] git-svn: fixes for intermittent SIGPIPE","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-04-24T09:45:41Z","receivedAt":"2012-04-24T09:45:41Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Roman Kagan wrote:\n> This is the third version of the series; compared to the previous submission\n> the log message of the first patch was reworded to make it more palatable.\n> \n> Roman Kagan (3):\n> git-svn: use POSIX::sigprocmask to block signals\n> git-svn: ignore SIGPIPE\n> git-svn: drop redundant blocking of SIGPIPE\n\nThanks for this series, acked and pushed to git://bogomips.org/git-svn\n"},{"id":"189983","messageId":"xmqq62cpgd50.fsf@junio.mtv.corp.google.com","threadId":"30319","inReplyTo":"20120424094541.GA30803@dcvr.yhbt.net","subject":"Re: [PATCH 0/3] git-svn: fixes for intermittent SIGPIPE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-24T19:09:47Z","receivedAt":"2012-04-24T19:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Roman Kagan wrote:\n>> This is the third version of the series; compared to the previous submission\n>> the log message of the first patch was reworded to make it more palatable.\n>> \n>> Roman Kagan (3):\n>> git-svn: use POSIX::sigprocmask to block signals\n>> git-svn: ignore SIGPIPE\n>> git-svn: drop redundant blocking of SIGPIPE\n>\n> Thanks for this series, acked and pushed to git://bogomips.org/git-svn\n\nThanks, both. I agree that the first one is much more nicely explained\nthan the previous one.\n"}]}