Community
Participate
Working Groups
The problem ------------ One of our git daemons has a problem and errors when a client pushes to an inexistent repo, throwing: ``` java.io.EOFException: Short read of block. ``` This exception is then caught by jgit and a `NoRemoteRepositoryException` is constructed but does not propagate to the caller. What is the expected output? ------------ The client (either gerrit or jgit standalone) should receive the NoRemoteRepositoryException. What do you see instead? ------------ The client (either Gerrit or jgit standalone) receives the following exception instead: ``` Caused by: org.eclipse.jgit.api.errors.TransportException: Can't overwrite cause with java.io.EOFException: Short read of block. ``` Please provide any additional information below. The issue, introduced here [1], is that jgit attempts to call `initCause()` on an exception that already has a cause. This, in turn, raises an IllegalStateException: ``` Caused by: java.lang.IllegalStateException: Can't overwrite cause with java.io.EOFException: Short read of block. ``` The consequence of this issue is that Gerrit (specifically, the replication plugin) fails to recognize that the exception was raised due to a `NoRemoteRepositoryException` and thus does not trigger the creation of the remote repository (see [2] for more details). Here is the full stack trace: ``` org.eclipse.jgit.errors.TransportException: Can't overwrite cause with java.io.EOFException: Short read of block. at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefs(BasePackConnection.java:185) at org.eclipse.jgit.transport.TransportGitAnon$TcpPushConnection.<init>(TransportGitAnon.java:234) at org.eclipse.jgit.transport.TransportGitAnon.openPush(TransportGitAnon.java:108) at org.eclipse.jgit.transport.PushProcess.execute(PushProcess.java:127) at org.eclipse.jgit.transport.Transport.push(Transport.java:1384) at org.eclipse.jgit.transport.Transport.push(Transport.java:1430) at com.googlesource.gerrit.plugins.replication.PushOne.pushInBatches(PushOne.java:576) at com.googlesource.gerrit.plugins.replication.PushOne.pushVia(PushOne.java:569) at com.googlesource.gerrit.plugins.replication.PushOne.runImpl(PushOne.java:540) at com.googlesource.gerrit.plugins.replication.PushOne.doRunPushOperation(PushOne.java:423) at com.googlesource.gerrit.plugins.replication.PushOne.runPushOperation(PushOne.java:391) at com.googlesource.gerrit.plugins.replication.PushOne.lambda$run$2(PushOne.java:377) at com.google.gerrit.server.util.RequestScopePropagator.lambda$cleanup$1(RequestScopePropagator.java:186) at com.google.gerrit.server.util.RequestScopePropagator.lambda$context$0(RequestScopePropagator.java:174) at com.google.gerrit.server.git.PerThreadRequestScope$Propagator.lambda$scope$0(PerThreadRequestScope.java:70) at com.googlesource.gerrit.plugins.replication.PushOne.run(PushOne.java:380) at com.google.gerrit.server.logging.LoggingContextAwareRunnable.run(LoggingContextAwareRunnable.java:95) at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:612) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829) Caused by: java.lang.IllegalStateException: Can't overwrite cause with java.io.EOFException: Short read of block. at java.base/java.lang.Throwable.initCause(Throwable.java:462) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefsImpl(BasePackConnection.java:214) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefs(BasePackConnection.java:179) ... 23 more Caused by: org.eclipse.jgit.errors.NoRemoteRepositoryException: REPO: not found. at org.eclipse.jgit.transport.BasePackConnection.noRepository(BasePackConnection.java:574) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefsImpl(BasePackConnection.java:213) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefs(BasePackConnection.java:179) at org.eclipse.jgit.transport.TransportGitAnon$TcpFetchConnection.<init>(TransportGitAnon.java:193) at org.eclipse.jgit.transport.TransportGitAnon$TcpFetchConnection.<init>(TransportGitAnon.java:168) at org.eclipse.jgit.transport.TransportGitAnon.openFetch(TransportGitAnon.java:95) at org.eclipse.jgit.transport.BasePackPushConnection.noRepository(BasePackPushConnection.java:151) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefsImpl(BasePackConnection.java:213) ... 24 more Caused by: java.io.EOFException: Short read of block. at org.eclipse.jgit.util.IO.readFully(IO.java:203) at org.eclipse.jgit.transport.PacketLineIn.readLength(PacketLineIn.java:316) at org.eclipse.jgit.transport.PacketLineIn.readString(PacketLineIn.java:180) at org.eclipse.jgit.transport.BasePackConnection.readLine(BasePackConnection.java:190) at org.eclipse.jgit.transport.BasePackConnection.readAdvertisedRefsImpl(BasePackConnection.java:211) ... 30 more ``` [1]https://git.eclipse.org/r/c/jgit/jgit/+/172595/9/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java#214 [2] https://bugs.chromium.org/p/gerrit/issues/detail?id=15538
Oi, that's convoluted. BasePackPushConnection overrides noRepository() and initiates a fetch, which may throw an already configured NoRemoteRepositoryException. Nasty. Something like if (noRepo.getCause() == null) { noRepo.initCause(e); } else { noRepo.addSuppressed(e); } should resolve this, shouldn't it?
Yes @Thomas, that's exactly what happens, we got to the same conclusion and to a very similar fix. We wanted first to have a failing test to surface the problem [1]. Likely today we'll push a fix. Also, looking at the existing code, this might not be the only case where we blindly `initCause` without first checking whether the exception already has one, so it might not be an isolated case. Once we get this out perhaps we should be looking at other possible occurrences. [1]https://git.eclipse.org/r/c/jgit/jgit/+/190250
All the other use of initCause() are fine. They all do Throwable t = new SomeException(someMessage); t.initCause(e); throw t; There are two more that do Throwable t = someMethodReturningAnException(); t.initCause(e); throw t; but in those two cases the method is private and does not set the cause. The one you found is the only case where the method is overrideable.
Yep, you're right, I also just scanned through the code and found no other problematic usage of initCause, good stuff. Let's just get this one fixed then.
Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/190250 was merged to [stable-5.13]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=78c9b9260a5287d09c87b407e396021590714513
