"Kyle J. McKay" <mackyle@xxxxxxxxx> wrote:
> On Oct 30, 2014, at 15:08, Eric Wong wrote:
> >For a (currently) unknown reason, Git::SVN::Fetcher::close_file
> >sometimes triggers "Bad file descriptor" errors on syswrite when
> >writing symlink contents on the "svn_hash" tempfile.
> >
> >The current IO::Handle::opened call in Git.pm is not a
> >sufficient check, as the underlying file descriptor is closed
> >without the PerlIO layer knowing about it. This is likely a bug
> >inside libsvn (1.6.17), as none of the Git.pm or Git::SVN
> >modules close IOs without the knowledge of the PerlIO layer.
> >
> >Cc: Kyle J. McKay <mackyle@xxxxxxxxx>
> >Signed-off-by: Eric Wong <normalperson@xxxxxxxx>
> >---
> >Kyle/Junio: thoughts? I'm running out of time to track this down
> >so it might be necessary for 2.2-rc0. What's even stranger is
> >I cannot always reproduce the problem even without this patch,
> >so it may be only triggered by network latency...
> >Thanks.
>
> With this patch added, do you then see the warning:
>
> carp "Temp file '", $name,
> "' was closed. Opening replacement.";
>
> From _temp_cache? It would seem odd if you didn't unless there was
> only one symlink involved.
Nope. $$temp_fd was defined so the message is not hit.
Is suppose we can also make the following change:
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1282,11 +1282,9 @@ sub _temp_cache {
$name . "' already in use");
}
} else {
- if (defined $$temp_fd) {
- # then we're here because of a closed handle.
- carp "Temp file '", $name,
- "' was closed. Opening replacement.";
- }
+ # then we're here because of a closed handle.
+ carp "Temp file '", $name,
+ "' was closed. Opening replacement.";
my $fname;
my $tmpdir;
> I suspect that symlinks were rare in the repositories I was testing
> against. I wonder if having a test svn repo that adds several new
> symlinks for several revisions in a row might trigger this problem
> more robustly.
>
> The _repository->temp_acquire('svn_hash') call is assigned to a "my"
> variable and then has Git::temp_release called on it later in the
> same function and the only calls made on it in between are
> Git::temp_path, so I don't see how the libsvn library could be
> responsible for closing it since it looks to me like it never sees
> it. But I'm looking at v2.1.2 of Fetcher.pm, so if some other calls
> have been inserted there since then.
I stared at it for a long while and could not figure out what was going
on, either. The other temp_acquire users are far less straightforward
and did not get mysteriously closed.
> The changes themselves look okay, but eeewwww. I don't see how
> libsvn can see the svn_hash fd to close it unless it's randomly
> closing fds.
Right. Even worse, I can't even get it to trigger anymore...
Thanks for your response!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html