Bugzilla – Full Text Bug Listing
| Summary: | Git wire protocol v2 is broken when running on JDK 15 | ||
|---|---|---|---|
| Product: | [Technology] JGit | Reporter: | David Ostrovsky <d.ostrovsky> |
| Component: | JGit | Assignee: | Project Inbox <jgit.core-inbox> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | Arthur.vanDorp, jonah, matthias.sohn, twolf |
| Version: | unspecified | ||
| Target Milestone: | 5.10 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| See Also: |
https://git.eclipse.org/r/c/jgit/jgit/+/172502 https://git.eclipse.org/c/jgit/jgit.git/commit/?id=7da0f0a8f37e35e9c3108588f1e6f7a7381d8f77 https://bugs.eclipse.org/bugs/show_bug.cgi?id=568904 https://git.eclipse.org/r/c/jgit/jgit/+/172535 https://git.eclipse.org/c/jgit/jgit.git/commit/?id=e2663a8b85cf92f6a84d72834257243a84066e9d https://git.eclipse.org/r/c/jgit/jgit/+/172601 https://git.eclipse.org/c/jgit/jgit.git/commit/?id=9f3616dcb43246b883f9943af0de7cf67836c443 https://git.eclipse.org/r/c/jgit/jgit/+/173012 https://git.eclipse.org/c/jgit/jgit.git/commit/?id=c053b510b3b921c5859a69bc74b98bcdec9c2a17 |
||
| Whiteboard: | |||
|
Description
David Ostrovsky
From bug 568904 comment 30: thomas@localhost:~/tmp/javatest$ more ot.java public class Ot { private static final String END = new StringBuilder(0).toString(); private static final String DELIM = new StringBuilder(0).toString(); public static void main(String... args) { System.out.println("END == DELIM = "+ Boolean.toString(END == DELIM)); } } thomas@localhost:~/tmp/javatest$ java -version openjdk version "11.0.1" 2018-10-16 LTS OpenJDK Runtime Environment 18.9 (build 11.0.1+13-LTS) OpenJDK 64-Bit Server VM 18.9 (build 11.0.1+13-LTS, mixed mode, sharing) thomas@localhost:~/tmp/javatest$ java ot.java END == DELIM = false thomas@localhost:~/tmp/javatest$ ../jdk15/jdk-15.0.1+9/bin/java --version openjdk 15.0.1 2020-10-20 OpenJDK Runtime Environment AdoptOpenJDK (build 15.0.1+9) OpenJDK 64-Bit Server VM AdoptOpenJDK (build 15.0.1+9, mixed mode, sharing) thomas@localhost:~/tmp/javatest$ ../jdk15/jdk-15.0.1+9/bin/java ot.java END == DELIM = true thomas@localhost:~/tmp/javatest$ So with Java 15, a basic assumption of JGit's PacketLineIn is violated. As a result, JGit cannot distinguish END and DELIM anymore, and parsing gets confused. Unclear to me whether that's a bug in Java 15. Seems to me that two different allocations should _never_ return the same object. If only because client code might use the object's monitors to synchronize something. But OK, JGit is relying here on the implementation of StringBuilder. If this is intended behavior in Java 15, PacketLineIn needs to find a different way to distinguish the two. This works as expected:
public class Ot {
private static String END = new String();
private static String DELIM = new String();
public static void main(String... args) {
System.out.println("END == DELIM = "+ Boolean.toString(END == DELIM));
}
}
END != DELIM, also on Java 15.
Why was to done with new StringBuilder(0).toString() at all? There's a comment in PacketLineIn "must not string pool"... why not?
The StringBuilder construction comes from JGit commit 08a9682e3 "Revert "[findBugs] Silence DM_STRING_CTOR on PacketLineIn"" (2010-11-09). Before that it was actually new String("").
(In reply to Thomas Wolf from comment #1) > So with Java 15, a basic assumption of JGit's PacketLineIn is violated. The assumption was wrong. > Unclear to me whether that's a bug in Java 15. Seems to me that two > different allocations should _never_ return the same object. Yes, but private static final String END = new StringBuilder(0).toString(); private static final String DELIM = new StringBuilder(0).toString(); are *not* two different allocations, but two calls to toString() method that never had any contract to return some specific object, just String representation of the buffer. That *should* return two *different* objects: private static final String END = new String(""); private static final String DELIM = new String(""); So I think this can be fixed in JGit. New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/172502 Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/172502 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=7da0f0a8f37e35e9c3108588f1e6f7a7381d8f77 New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/172535 Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/172535 was merged to [stable-5.9]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=e2663a8b85cf92f6a84d72834257243a84066e9d New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/172601 Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/172601 was merged to [stable-5.8]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=9f3616dcb43246b883f9943af0de7cf67836c443 Thanks folks for fixing this Java 15 related bug so quickly. Did this bug exist and is it now fixed in simrel's version of jgit? The reason I ask is that the current plan is to ship EPP packages and Eclipse installer with Java 15 bundles for 2020-12. The 2020-12 M2 had Java 14, but M3 will have Java 15. (In reply to Jonah Graham from comment #10) > Thanks folks for fixing this Java 15 related bug so quickly. > Did this bug exist and is it now fixed in simrel's version of jgit? The > reason I ask is that the current plan is to ship EPP packages and Eclipse > installer with Java 15 bundles for 2020-12. The 2020-12 M2 had Java 14, but > M3 will have Java 15. Yes. The bug was fixed in JGit in the main branch: [1] and the fix was cherry-picked to stable-5.8: [2] and stable-5.9 branches: [3]. [1] https://git.eclipse.org/c/jgit/jgit.git/commit/?id=7da0f0a8f37e35e9c3108588f1e6f7a7381d8f77 [2] https://git.eclipse.org/c/jgit/jgit.git/commit/?id=9f3616dcb43246b883f9943af0de7cf67836c443 [3] https://git.eclipse.org/c/jgit/jgit.git/commit/?id=e2663a8b85cf92f6a84d72834257243a84066e9d (In reply to David Ostrovsky from comment #11) > (In reply to Jonah Graham from comment #10) > > Did this bug exist and is it now fixed in simrel's version of jgit? > > M3 will have Java 15. > > Yes. The bug was fixed in JGit in the main branch Matthias will need to create 5.10.0-m3 releases of JGit and EGit and update the simrel aggregator to pick those up, though. And this would need to happen today or tomorrow. Don't know if he has the time for this. However, this bug surfaces only in protocol V2, and the JGit client side has no implementation for protocol V2 yet but always uses protocol V0, and Eclipse only uses the JGit client side. So even if we don't make it for M3 that shouldn't be a problem. The final 2020-12 release will include a JGit containing this fix for sure. (In reply to Thomas Wolf from comment #12) > (In reply to David Ostrovsky from comment #11) > > (In reply to Jonah Graham from comment #10) > > > Did this bug exist and is it now fixed in simrel's version of jgit? > > > M3 will have Java 15. > > > > Yes. The bug was fixed in JGit in the main branch > > Matthias will need to create 5.10.0-m3 releases of JGit and EGit and update > the simrel aggregator to pick those up, though. And this would need to > happen today or tomorrow. I plan to contribute the latest version which includes the fix to m3 today or tomorrow > Don't know if he has the time for this. However, this bug surfaces only in > protocol V2, and the JGit client side has no implementation for protocol V2 > yet but always uses protocol V0, and Eclipse only uses the JGit client side. > So even if we don't make it for M3 that shouldn't be a problem. > > The final 2020-12 release will include a JGit containing this fix for sure. New Gerrit change created: https://git.eclipse.org/r/c/jgit/jgit/+/173012 Gerrit change https://git.eclipse.org/r/c/jgit/jgit/+/173012 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=c053b510b3b921c5859a69bc74b98bcdec9c2a17 |