# git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor 14 messages from 2015-01-29 to 2015-02-25. Participants: Valery Yundin, Eric Wong, Nico Schlömer, Kyle J. McKay. Thread: https://gitlist.dev/t/38471 ## Valery Yundin, 2015-01-29 22:03 Subject: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CABNxngNDwf_Cy77OzvMg__kCNoTz5y1a2KKG1vobYjE_m_aLkQ%40mail.gmail.com ``` Hi, Looks like there might be a bug in SVN import code. Here is the command that fails > git svn clone --username anonymous svn://powhegbox.mib.infn.it/trunk/POWHEG-BOX r217 = 2e6cdb1f4604b2256195590fa8275632971eac42 (refs/remotes/git-svn) M lhefread.f M Z_plus_jet/Born.f M Z_plus_jet/powheg.input M Z_plus_jet/init_processes.f D JJ/madgraph_3_flavours/born/nexternal.inc write .git/Git_svn_hash_bl5Cj3: Bad file descriptor at /usr/lib/perl5/vendor_perl/5.20.1/x86_64-linux-thread-multi/SVN/Ra.pm line 623. Tested on: git-svn version 2.3.0.rc2 (svn 1.8.11) - FAIL git-svn version 2.2.2 (svn 1.8.10) - FAIL git-svn version 1.8.4.5 (svn 1.8.11) - WORKS PS unfortunately I don't have admin access to SVN repository With best regards, Valery Yundin. ``` ## Eric Wong, 2015-01-29 23:34 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150129233429.GA7162@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150129233429.GA7162%40dcvr.yhbt.net In-Reply-To: ``` Valery Yundin wrote: > Tested on: > git-svn version 2.3.0.rc2 (svn 1.8.11) - FAIL > git-svn version 2.2.2 (svn 1.8.10) - FAIL > git-svn version 1.8.4.5 (svn 1.8.11) - WORKS Thank you for that info. Do you think you can bisect which versions/revisions of git-svn introduced that failure for us? I don't have much time this part of the year for git-svn, but maybe it's related to the performance work we did around fall 2014. Thanks again. ``` ## Valery Yundin, 2015-01-29 23:59 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CABNxngPErFiTzUNK6P90Ug5fVtJSYD9XrGrQzK8%2Bmt2u4g%3DXyg%40mail.gmail.com In-Reply-To: <20150129233429.GA7162@dcvr.yhbt.net> ``` Hi, Here you go: dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit commit dfa72fdb96befbd790f623bb2909a347176753c2 Author: Eric Wong Date: Fri Oct 24 22:53:52 2014 +0000 git-svn: reload RA every log-window-size Despite attempting to use local memory pools everywhere we can, (including our call to SVN::Ra::do_update and all subsequent reporter calls), there does not appear to be a way to force the Git::SVN::Fetcher callbacks to use a pool other than the per-SVN::Ra pool. Git::SVN::Fetcher ends up using the main RA pool which grows monotonically in size for the lifetime of the RA object. Thus the only way to free that memory appears to be to destroy and recreate the RA connection for at every --log-window-size interval. This reduces memory usage over the course of fetching 10K revisions using a test repository created with the script at the end of this commit message. ......... Cheers, Valery With best regards, Mr. Valery Yundin. On 30 January 2015 at 00:34, Eric Wong wrote: > Valery Yundin wrote: >> Tested on: >> git-svn version 2.3.0.rc2 (svn 1.8.11) - FAIL >> git-svn version 2.2.2 (svn 1.8.10) - FAIL >> git-svn version 1.8.4.5 (svn 1.8.11) - WORKS > > Thank you for that info. Do you think you can bisect which > versions/revisions of git-svn introduced that failure for us? I don't > have much time this part of the year for git-svn, but maybe it's related > to the performance work we did around fall 2014. > > Thanks again. ``` ## Eric Wong, 2015-01-30 00:22 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150130002247.GA22519@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150130002247.GA22519%40dcvr.yhbt.net In-Reply-To: ``` Valery Yundin wrote: > Hi, > > Here you go: > dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit Thank you. Can you give the following patch a try? I have not been able to reproduce the problem on my end. If it doesn't work out, I might be out of ideas for a bit :/ Increasing --log-window-size will help you run longer without the error, but that's not ideal as it can also eat memory. -----------------------8<---------------------- From: Eric Wong Subject: [PATCH] git-svn: destroy all tempfiles when reloading RA This may fix the errors some users are seeing with: "write .git/Git_svn_hash_XXXXXX: Bad file descriptor" Thanks to Valery Yundin for helping bisect the problem introduced in commit dfa72fdb96befbd790f623bb2909a347176753c2 (git-svn: reload RA every log-window-size) Cc: Valery Yundin Signed-off-by: Eric Wong --- perl/Git.pm | 6 ++++++ perl/Git/SVN/Ra.pm | 1 + 2 files changed, 7 insertions(+) diff --git a/perl/Git.pm b/perl/Git.pm index b5905ee..698018e 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -1347,6 +1347,12 @@ sub temp_path { $TEMP_FILES{$temp_fd}{fname}; } +sub temp_reset_all { + unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; + %TEMP_FILEMAP = (); + %TEMP_FILES = (); +} + sub END { unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; } diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 622535e..878679d 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -397,6 +397,7 @@ sub gs_fetch_loop_common { $_[0] = undef; $self = undef; $RA = undef; + Git->temp_reset_all; $gpool->clear; $self = Git::SVN::Ra->new($ra_url); $ra_invalid = undef; -- EW ``` ## Valery Yundin, 2015-01-30 00:40 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CABNxngMVsaSLmzf0ZsaXZqXhD%2BhOaRmz-uuSzm0ALDFhKjnrXA%40mail.gmail.com In-Reply-To: <20150130002247.GA22519@dcvr.yhbt.net> ``` Hi, Your patch seems to fix the problem. I tried several times and I can svn clone the whole repository in one go without a crash. Thanks, Valery On 30 January 2015 at 01:22, Eric Wong wrote: > Valery Yundin wrote: >> Hi, >> >> Here you go: >> dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit > > Thank you. Can you give the following patch a try? > I have not been able to reproduce the problem on my end. > If it doesn't work out, I might be out of ideas for a bit :/ > Increasing --log-window-size will help you run longer without > the error, but that's not ideal as it can also eat memory. > > -----------------------8<---------------------- > From: Eric Wong > Subject: [PATCH] git-svn: destroy all tempfiles when reloading RA > > This may fix the errors some users are seeing with: > "write .git/Git_svn_hash_XXXXXX: Bad file descriptor" > > Thanks to Valery Yundin for helping bisect the problem introduced in > commit dfa72fdb96befbd790f623bb2909a347176753c2 > (git-svn: reload RA every log-window-size) > > Cc: Valery Yundin > Signed-off-by: Eric Wong > --- > perl/Git.pm | 6 ++++++ > perl/Git/SVN/Ra.pm | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/perl/Git.pm b/perl/Git.pm > index b5905ee..698018e 100644 > --- a/perl/Git.pm > +++ b/perl/Git.pm > @@ -1347,6 +1347,12 @@ sub temp_path { > $TEMP_FILES{$temp_fd}{fname}; > } > > +sub temp_reset_all { > + unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; > + %TEMP_FILEMAP = (); > + %TEMP_FILES = (); > +} > + > sub END { > unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; > } > diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm > index 622535e..878679d 100644 > --- a/perl/Git/SVN/Ra.pm > +++ b/perl/Git/SVN/Ra.pm > @@ -397,6 +397,7 @@ sub gs_fetch_loop_common { > $_[0] = undef; > $self = undef; > $RA = undef; > + Git->temp_reset_all; > $gpool->clear; > $self = Git::SVN::Ra->new($ra_url); > $ra_invalid = undef; > -- > EW ``` ## Eric Wong, 2015-01-30 01:30 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150130013017.GA10713@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150130013017.GA10713%40dcvr.yhbt.net In-Reply-To: ``` Valery Yundin wrote: > Hi, > > Your patch seems to fix the problem. > I tried several times and I can svn clone the whole repository in one > go without a crash. Thanks for the confirmation. Cc-ing a few other folks who encountered this problem (and Bcc-ing some folks who emailed me privately). Can the rest of you give this patch a try on your respective platforms and confirm the fix? http://article.gmane.org/gmane.comp.version-control.git/263168/raw (also: http://mid.gmane.org/20150130002247.GA22519@dcvr.yhbt.net ) Junio: assuming all goes well with testers, can you apply my patch to the appropriate maintenance tracks with Tested-by:s? I'm going offline in a few hours and don't think I'll be around much the next week or so. Thanks. > Thanks, > Valery > > On 30 January 2015 at 01:22, Eric Wong wrote: > > Valery Yundin wrote: > >> Hi, > >> > >> Here you go: > >> dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit > > > > Thank you. Can you give the following patch a try? > > I have not been able to reproduce the problem on my end. > > If it doesn't work out, I might be out of ideas for a bit :/ > > Increasing --log-window-size will help you run longer without > > the error, but that's not ideal as it can also eat memory. > > > > -----------------------8<---------------------- > > From: Eric Wong > > Subject: [PATCH] git-svn: destroy all tempfiles when reloading RA > > > > This may fix the errors some users are seeing with: > > "write .git/Git_svn_hash_XXXXXX: Bad file descriptor" > > > > Thanks to Valery Yundin for helping bisect the problem introduced in > > commit dfa72fdb96befbd790f623bb2909a347176753c2 > > (git-svn: reload RA every log-window-size) > > > > Cc: Valery Yundin > > Signed-off-by: Eric Wong > > --- > > perl/Git.pm | 6 ++++++ > > perl/Git/SVN/Ra.pm | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/perl/Git.pm b/perl/Git.pm > > index b5905ee..698018e 100644 > > --- a/perl/Git.pm > > +++ b/perl/Git.pm > > @@ -1347,6 +1347,12 @@ sub temp_path { > > $TEMP_FILES{$temp_fd}{fname}; > > } > > > > +sub temp_reset_all { > > + unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; > > + %TEMP_FILEMAP = (); > > + %TEMP_FILES = (); > > +} > > + > > sub END { > > unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; > > } > > diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm > > index 622535e..878679d 100644 > > --- a/perl/Git/SVN/Ra.pm > > +++ b/perl/Git/SVN/Ra.pm > > @@ -397,6 +397,7 @@ sub gs_fetch_loop_common { > > $_[0] = undef; > > $self = undef; > > $RA = undef; > > + Git->temp_reset_all; > > $gpool->clear; > > $self = Git::SVN::Ra->new($ra_url); > > $ra_invalid = undef; > > -- > > EW > -- ``` ## Nico Schlömer, 2015-01-31 12:51 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CAK6Z60cGnvPP0ctnyV4tHfMmBGYRidEJzkAG%2BJ2h9BunmtOAsQ%40mail.gmail.com In-Reply-To: <20150130013017.GA10713@dcvr.yhbt.net> ``` I tried the patch and I still get ˋˋˋ [...] r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) Unexpected result returned from git cat-file at /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335. Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, line 757. error closing pipe: Bad file descriptor at /home/nschloe/libexec/git-core/git-svn line 0. error closing pipe: Bad file descriptor at /home/nschloe/libexec/git-core/git-svn line 0. ˋˋˋ when ˋˋˋ git svn clone https://geuz.org/svn/gmsh/trunk ˋˋˋ Cheers, Nico On Fri, Jan 30, 2015 at 2:30 AM, Eric Wong wrote: > Valery Yundin wrote: >> Hi, >> >> Your patch seems to fix the problem. >> I tried several times and I can svn clone the whole repository in one >> go without a crash. > > Thanks for the confirmation. Cc-ing a few other folks who encountered > this problem (and Bcc-ing some folks who emailed me privately). > > Can the rest of you give this patch a try on your respective platforms > and confirm the fix? > > http://article.gmane.org/gmane.comp.version-control.git/263168/raw > (also: http://mid.gmane.org/20150130002247.GA22519@dcvr.yhbt.net ) > > Junio: assuming all goes well with testers, can you apply my patch > to the appropriate maintenance tracks with Tested-by:s? > I'm going offline in a few hours and don't think I'll be around > much the next week or so. > > Thanks. > >> Thanks, >> Valery >> >> On 30 January 2015 at 01:22, Eric Wong wrote: >> > Valery Yundin wrote: >> >> Hi, >> >> >> >> Here you go: >> >> dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit >> > >> > Thank you. Can you give the following patch a try? >> > I have not been able to reproduce the problem on my end. >> > If it doesn't work out, I might be out of ideas for a bit :/ >> > Increasing --log-window-size will help you run longer without >> > the error, but that's not ideal as it can also eat memory. >> > >> > -----------------------8<---------------------- >> > From: Eric Wong >> > Subject: [PATCH] git-svn: destroy all tempfiles when reloading RA >> > >> > This may fix the errors some users are seeing with: >> > "write .git/Git_svn_hash_XXXXXX: Bad file descriptor" >> > >> > Thanks to Valery Yundin for helping bisect the problem introduced in >> > commit dfa72fdb96befbd790f623bb2909a347176753c2 >> > (git-svn: reload RA every log-window-size) >> > >> > Cc: Valery Yundin >> > Signed-off-by: Eric Wong >> > --- >> > perl/Git.pm | 6 ++++++ >> > perl/Git/SVN/Ra.pm | 1 + >> > 2 files changed, 7 insertions(+) >> > >> > diff --git a/perl/Git.pm b/perl/Git.pm >> > index b5905ee..698018e 100644 >> > --- a/perl/Git.pm >> > +++ b/perl/Git.pm >> > @@ -1347,6 +1347,12 @@ sub temp_path { >> > $TEMP_FILES{$temp_fd}{fname}; >> > } >> > >> > +sub temp_reset_all { >> > + unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; >> > + %TEMP_FILEMAP = (); >> > + %TEMP_FILES = (); >> > +} >> > + >> > sub END { >> > unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; >> > } >> > diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm >> > index 622535e..878679d 100644 >> > --- a/perl/Git/SVN/Ra.pm >> > +++ b/perl/Git/SVN/Ra.pm >> > @@ -397,6 +397,7 @@ sub gs_fetch_loop_common { >> > $_[0] = undef; >> > $self = undef; >> > $RA = undef; >> > + Git->temp_reset_all; >> > $gpool->clear; >> > $self = Git::SVN::Ra->new($ra_url); >> > $ra_invalid = undef; >> > -- >> > EW >> -- ``` ## Valery Yundin, 2015-01-31 14:52 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CABNxngNH20O52QHSivQLvRFSW2hqnVTYuBv0CWn1L53cL%2BXoGw%40mail.gmail.com In-Reply-To: ``` Hi, It seems that the same commit dfa72fdb96 is responsible for the error in "git svn clone https://geuz.org/svn/gmsh/trunk". But unlike in my case, the patch doesn't fix it. Cheers, Valery On 31 January 2015 at 13:51, Nico Schlömer wrote: > I tried the patch and I still get > ˋˋˋ > [...] > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) > Unexpected result returned from git cat-file at > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335. > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, > line 757. > > error closing pipe: Bad file descriptor at > /home/nschloe/libexec/git-core/git-svn line 0. > error closing pipe: Bad file descriptor at > /home/nschloe/libexec/git-core/git-svn line 0. > ˋˋˋ > when > ˋˋˋ > git svn clone https://geuz.org/svn/gmsh/trunk > ˋˋˋ > > Cheers, > Nico > > On Fri, Jan 30, 2015 at 2:30 AM, Eric Wong wrote: >> Valery Yundin wrote: >>> Hi, >>> >>> Your patch seems to fix the problem. >>> I tried several times and I can svn clone the whole repository in one >>> go without a crash. >> >> Thanks for the confirmation. Cc-ing a few other folks who encountered >> this problem (and Bcc-ing some folks who emailed me privately). >> >> Can the rest of you give this patch a try on your respective platforms >> and confirm the fix? >> >> http://article.gmane.org/gmane.comp.version-control.git/263168/raw >> (also: http://mid.gmane.org/20150130002247.GA22519@dcvr.yhbt.net ) >> >> Junio: assuming all goes well with testers, can you apply my patch >> to the appropriate maintenance tracks with Tested-by:s? >> I'm going offline in a few hours and don't think I'll be around >> much the next week or so. >> >> Thanks. >> >>> Thanks, >>> Valery >>> >>> On 30 January 2015 at 01:22, Eric Wong wrote: >>> > Valery Yundin wrote: >>> >> Hi, >>> >> >>> >> Here you go: >>> >> dfa72fdb96befbd790f623bb2909a347176753c2 is the first bad commit >>> > >>> > Thank you. Can you give the following patch a try? >>> > I have not been able to reproduce the problem on my end. >>> > If it doesn't work out, I might be out of ideas for a bit :/ >>> > Increasing --log-window-size will help you run longer without >>> > the error, but that's not ideal as it can also eat memory. >>> > >>> > -----------------------8<---------------------- >>> > From: Eric Wong >>> > Subject: [PATCH] git-svn: destroy all tempfiles when reloading RA >>> > >>> > This may fix the errors some users are seeing with: >>> > "write .git/Git_svn_hash_XXXXXX: Bad file descriptor" >>> > >>> > Thanks to Valery Yundin for helping bisect the problem introduced in >>> > commit dfa72fdb96befbd790f623bb2909a347176753c2 >>> > (git-svn: reload RA every log-window-size) >>> > >>> > Cc: Valery Yundin >>> > Signed-off-by: Eric Wong >>> > --- >>> > perl/Git.pm | 6 ++++++ >>> > perl/Git/SVN/Ra.pm | 1 + >>> > 2 files changed, 7 insertions(+) >>> > >>> > diff --git a/perl/Git.pm b/perl/Git.pm >>> > index b5905ee..698018e 100644 >>> > --- a/perl/Git.pm >>> > +++ b/perl/Git.pm >>> > @@ -1347,6 +1347,12 @@ sub temp_path { >>> > $TEMP_FILES{$temp_fd}{fname}; >>> > } >>> > >>> > +sub temp_reset_all { >>> > + unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; >>> > + %TEMP_FILEMAP = (); >>> > + %TEMP_FILES = (); >>> > +} >>> > + >>> > sub END { >>> > unlink values %TEMP_FILEMAP if %TEMP_FILEMAP; >>> > } >>> > diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm >>> > index 622535e..878679d 100644 >>> > --- a/perl/Git/SVN/Ra.pm >>> > +++ b/perl/Git/SVN/Ra.pm >>> > @@ -397,6 +397,7 @@ sub gs_fetch_loop_common { >>> > $_[0] = undef; >>> > $self = undef; >>> > $RA = undef; >>> > + Git->temp_reset_all; >>> > $gpool->clear; >>> > $self = Git::SVN::Ra->new($ra_url); >>> > $ra_invalid = undef; >>> > -- >>> > EW >>> -- ``` ## Eric Wong, 2015-02-12 19:18 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150212191807.GA30062@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150212191807.GA30062%40dcvr.yhbt.net In-Reply-To: ``` Valery Yundin wrote: > On 31 January 2015 at 13:51, Nico Schlömer wrote: > > I tried the patch and I still get > > ˋˋˋ > > [...] > > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) > > Unexpected result returned from git cat-file at > > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335. > > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at > > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, > > line 757. > > > > error closing pipe: Bad file descriptor at > > /home/nschloe/libexec/git-core/git-svn line 0. > > error closing pipe: Bad file descriptor at > > /home/nschloe/libexec/git-core/git-svn line 0. > > ˋˋˋ > > when > > ˋˋˋ > > git svn clone https://geuz.org/svn/gmsh/trunk > > It seems that the same commit dfa72fdb96 is responsible for the error > in "git svn clone https://geuz.org/svn/gmsh/trunk". But unlike in my > case, the patch doesn't fix it. (top-posting corrected) Odd, I managed to clone that without issues, but I couldn't reproduce this problem with or without the tempfile clearing patch applied. git svn clone --username gmsh https://geuz.org/svn/gmsh/trunk Anybody else? ``` ## Nico Schlömer, 2015-02-16 15:10 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: URL: https://gitlist.dev/e/CAK6Z60dZu_0kNJF2-eJMgnjYVhxfyps53mcfZT41V1DBZZ%2Bzhw%40mail.gmail.com In-Reply-To: <20150212191807.GA30062@dcvr.yhbt.net> ``` I just double-checked and I can only produce this issue on one machine (tested on 3). Apparently, this is has nothing to do with Git itself then. Any ideas of what could be wrong? Cheers, Nico On Thu, Feb 12, 2015 at 8:18 PM, Eric Wong wrote: > Valery Yundin wrote: >> On 31 January 2015 at 13:51, Nico Schlömer wrote: >> > I tried the patch and I still get >> > ˋˋˋ >> > [...] >> > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) >> > Unexpected result returned from git cat-file at >> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335. >> > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at >> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, >> > line 757. >> > >> > error closing pipe: Bad file descriptor at >> > /home/nschloe/libexec/git-core/git-svn line 0. >> > error closing pipe: Bad file descriptor at >> > /home/nschloe/libexec/git-core/git-svn line 0. >> > ˋˋˋ >> > when >> > ˋˋˋ >> > git svn clone https://geuz.org/svn/gmsh/trunk >> >> It seems that the same commit dfa72fdb96 is responsible for the error >> in "git svn clone https://geuz.org/svn/gmsh/trunk". But unlike in my >> case, the patch doesn't fix it. > > (top-posting corrected) > > Odd, I managed to clone that without issues, but I couldn't reproduce > this problem with or without the tempfile clearing patch applied. > > git svn clone --username gmsh https://geuz.org/svn/gmsh/trunk > > Anybody else? ``` ## Eric Wong, 2015-02-17 06:17 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150217061707.GA4308@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150217061707.GA4308%40dcvr.yhbt.net In-Reply-To: ``` Nico Schlömer wrote: > I just double-checked and I can only produce this issue on one machine > (tested on 3). Apparently, this is has nothing to do with Git itself > then. > > Any ideas of what could be wrong? What's different about that one machine? e.g. SVN version, 64 vs 32-bit, Perl version, etc. could all be factors (assuming identical git versions). Also, any chance git was misinstalled somehow or your PATH was not pointing to the correct git installation? Thanks ``` ## Kyle J. McKay, 2015-02-25 10:19 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <801181a93d829d68b96c4b61d1ebdc3@74d39fa044aa309eaea14b9f57fe79c> URL: https://gitlist.dev/e/801181a93d829d68b96c4b61d1ebdc3%4074d39fa044aa309eaea14b9f57fe79c In-Reply-To: <20150217061707.GA4308@dcvr.yhbt.net> ``` I believe I have been able to track down this problem and implement a fix. Please report back if this patch fixes the problem for you. -Kyle -- 8< -- Subject: [PATCH] Git::SVN::Fetcher: avoid premature 'svn_hash' temp file closure Since b19138b (git-svn: Make it incrementally faster by minimizing temp files, v1.6.0), git-svn has been using the Git.pm temp_acquire and temp_release mechanism to avoid unnecessary temp file churn and provide a speed boost. However, that change introduced a call to temp_acquire inside the Git::SVN::Fetcher::close_file function for an 'svn_hash' temp file. Because an SVN::Pool is active at the time this function is called, if the Git::temp_acquire function ends up actually creating a new FileHandle for the temp file (which it will the first time it's called with the name 'svn_hash') that FileHandle will end up in the SVN::Pool and should that pool have SVN::Pool::clear called on it that FileHandle will be closed out from under Git::temp_acquire. Since the only call site to Git::temp_acquire with the name 'svn_hash' is inside the close_file function, if an 'svn_hash' temp file is ever created its FileHandle is guaranteed to be created in the active SVN::Pool. This has not been a problem in the past because the SVN::Pool was not being cleared. However, since dfa72fdb (git-svn: reload RA every log-window-size, v2.2.0) the pool has been getting cleared periodically at which point the FileHandle for the 'svn_hash' temp file gets closed. Any subsequent calls to Git::temp_acquire for 'svn_hash', however, succeed without creating/opening a new temporary file since it still has the now invalid FileHandle in its cache. Callers that then attempt to use that FileHandle fail with an error. We avoid this problem by making sure the 'svn_hash' temp file is created in the same place the 'svn_delta_...' and 'git_blob_...' temp files are (and then temp_release'd) so that it can be safely used inside the close_file function without having its FileHandle end up in an SVN::Pool that gets cleared. Signed-off-by: Kyle J. McKay --- perl/Git/SVN/Fetcher.pm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm index 10edb277..613055a3 100644 --- a/perl/Git/SVN/Fetcher.pm +++ b/perl/Git/SVN/Fetcher.pm @@ -322,6 +322,14 @@ sub apply_textdelta { # (but $base does not,) so dup() it for reading in close_file open my $dup, '<&', $fh or croak $!; my $base = $::_repository->temp_acquire("git_blob_${$}_$suffix"); + # close_file may call temp_acquire on 'svn_hash', but because of the + # call chain, if the temp_acquire call from close_file ends up being the + # call that first creates the 'svn_hash' temp file, then the FileHandle + # that's created as a result will end up in an SVN::Pool that we clear + # in SVN::Ra::gs_fetch_loop_common. Avoid that by making sure the + # 'svn_hash' FileHandle is already created before close_file is called. + my $tmp_fh = $::_repository->temp_acquire('svn_hash'); + $::_repository->temp_release($tmp_fh, 1); if ($fb->{blob}) { my ($base_is_link, $size); -- ``` ## Eric Wong, 2015-02-25 20:08 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <20150225200833.GA10031@dcvr.yhbt.net> URL: https://gitlist.dev/e/20150225200833.GA10031%40dcvr.yhbt.net In-Reply-To: ``` Nico Schlömer wrote: > Thanks Kyle for the patch! I applied it and ran > ˋˋˋ > git svn clone https://geuz.org/svn/gmsh/trunk > ˋˋˋ > Unfortunately, I'm still getting > ˋˋˋ > [...] > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) > error closing pipe: Bad file descriptor at > /usr/share/perl5/Git/SVN/Fetcher.pm line 335. > error closing pipe: Bad file descriptor at > /usr/share/perl5/Git/SVN/Fetcher.pm line 335. > out pipe went bad at /usr/share/perl5/Git.pm line 955. Thanks for testing, is this with or without my other attempt at fixing this problem applied? http://mid.gmane.org/20150130002247.GA22519@dcvr.yhbt.net ("git-svn: destroy all tempfiles when reloading RA") And can you try testing both with/without that patch if you haven't already? Thanks again. ``` ## Kyle J. McKay, 2015-02-25 20:33 Subject: Re: git svn import failure : write .git/Git_svn_hash_BmjclS: Bad file descriptor Message-ID: <8FAF534A-812E-4FBD-AEE4-907C83CEC007@gmail.com> URL: https://gitlist.dev/e/8FAF534A-812E-4FBD-AEE4-907C83CEC007%40gmail.com In-Reply-To: ``` On Feb 25, 2015, at 07:07, Nico Schlömer wrote: > Thanks Kyle for the patch! I applied it and ran > ˋˋˋ > git svn clone https://geuz.org/svn/gmsh/trunk > ˋˋˋ > Unfortunately, I'm still getting > ˋˋˋ > [...] > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/ > Fetcher.pm line 335. > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/ > Fetcher.pm line 335. > out pipe went bad at /usr/share/perl5/Git.pm line 955. > ˋˋˋ Are you certain you're running with the patch? I ask because earlier you sent this: On Jan 31, 2015, at 04:51, Nico Schlömer wrote: > I tried the patch and I still get > ˋˋˋ > [...] > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) > Unexpected result returned from git cat-file at > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335. > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, > line 757. > > error closing pipe: Bad file descriptor at > /home/nschloe/libexec/git-core/git-svn line 0. > error closing pipe: Bad file descriptor at > /home/nschloe/libexec/git-core/git-svn line 0. > ˋˋˋ > when > ˋˋˋ > git svn clone https://geuz.org/svn/gmsh/trunk > ˋˋˋ And as the patch below applies to Fetcher.pm at line 322 and inserts 8 lines, I do not see how you could be getting the same error message at line 335 if the patch was present. Even if you manually applied it by only inserting the two lines of code, the line numbers would be at least 337, not 335 as reported above. I also notice the path to Fetcher.pm is different from your earlier report. That said, the fact that it happens right after r100 (which is when SVN::Pool::clear is getting called) suggests there's another file handle getting caught up in the SVN::Pool somehow. (When I try to clone gmsh, I got to r4885 before a server disconnection.) It could be as simple as the open2 call FileHandle result in later perl versions ends up in the SVN::Pool whereas in earlier Perl and/or svn versions it does not. What happens if you add this change on top of the Fetcher.pm change: diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 622535e2..96888228 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -391,6 +391,7 @@ sub longest_common_path { sub gs_fetch_loop_common { my ($self, $base, $head, $gsv, $globs) = @_; return if ($base > $head); + $::_repository->_open_cat_blob_if_needed; my $gpool = SVN::Pool->new_default; my $ra_url = $self->url; my $reload_ra = sub { -Kyle > Cheers, > Nico > > On Wed, Feb 25, 2015 at 11:20 AM Kyle J. McKay > wrote: > I believe I have been able to track down this problem and implement a > fix. Please report back if this patch fixes the problem for you. > > -Kyle > > -- 8< -- > Subject: [PATCH] Git::SVN::Fetcher: avoid premature 'svn_hash' temp > file closure > > Since b19138b (git-svn: Make it incrementally faster by minimizing > temp > files, v1.6.0), git-svn has been using the Git.pm temp_acquire and > temp_release mechanism to avoid unnecessary temp file churn and > provide > a speed boost. > > However, that change introduced a call to temp_acquire inside the > Git::SVN::Fetcher::close_file function for an 'svn_hash' temp file. > Because an SVN::Pool is active at the time this function is called, if > the Git::temp_acquire function ends up actually creating a new > FileHandle for the temp file (which it will the first time it's called > with the name 'svn_hash') that FileHandle will end up in the SVN::Pool > and should that pool have SVN::Pool::clear called on it that > FileHandle > will be closed out from under Git::temp_acquire. > > Since the only call site to Git::temp_acquire with the name 'svn_hash' > is inside the close_file function, if an 'svn_hash' temp file is ever > created its FileHandle is guaranteed to be created in the active > SVN::Pool. > > This has not been a problem in the past because the SVN::Pool was not > being cleared. However, since dfa72fdb (git-svn: reload RA every > log-window-size, v2.2.0) the pool has been getting cleared > periodically > at which point the FileHandle for the 'svn_hash' temp file gets > closed. > Any subsequent calls to Git::temp_acquire for 'svn_hash', however, > succeed without creating/opening a new temporary file since it still > has > the now invalid FileHandle in its cache. Callers that then attempt to > use that FileHandle fail with an error. > > We avoid this problem by making sure the 'svn_hash' temp file is > created > in the same place the 'svn_delta_...' and 'git_blob_...' temp files > are > (and then temp_release'd) so that it can be safely used inside the > close_file function without having its FileHandle end up in an > SVN::Pool > that gets cleared. > > Signed-off-by: Kyle J. McKay > --- > perl/Git/SVN/Fetcher.pm | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm > index 10edb277..613055a3 100644 > --- a/perl/Git/SVN/Fetcher.pm > +++ b/perl/Git/SVN/Fetcher.pm > @@ -322,6 +322,14 @@ sub apply_textdelta { > # (but $base does not,) so dup() it for reading in close_file > open my $dup, '<&', $fh or croak $!; > my $base = $::_repository->temp_acquire("git_blob_${$}_ > $suffix"); > + # close_file may call temp_acquire on 'svn_hash', but > because of the > + # call chain, if the temp_acquire call from close_file ends > up being the > + # call that first creates the 'svn_hash' temp file, then the > FileHandle > + # that's created as a result will end up in an SVN::Pool > that we clear > + # in SVN::Ra::gs_fetch_loop_common. Avoid that by making > sure the > + # 'svn_hash' FileHandle is already created before close_file > is called. > + my $tmp_fh = $::_repository->temp_acquire('svn_hash'); > + $::_repository->temp_release($tmp_fh, 1); > > if ($fb->{blob}) { > my ($base_is_link, $size); > -- ```