jdk8 vs jdk 10

[oscarguindzberg]

The problem

bisq has to be built using jdk 10. bitcoinj has to be built using jdk 8.

Details

bitcoinj 0.14.4+ uses sun.misc.Cleaner.
sun.* packages should not be used directly as a rule of thumb.
bitcoinj uses them to fix some windows specific issue.
Using sun.* seemed to be the lesser evil.
The thing is sun.misc.Cleaner was removed in jdk 9.
bitcoinj does not compile if you use jdk 9, 10 or 11.

The problem was discussed on several bitcoinj mailing list msgs and github issues. The most relevant discussion happened here: bitcoinj#1477

Andreas Schildbach eventually removed the windows specific hack, which eliminates the need to use sun.misc.Cleaner (he thinks the latest versions of windows may not reproduce the bug)
Fix:
bitcoinj@1700ec0
Test:
bitcoinj@dbc4c35
Andreas Schildbach did the change on bitcoinj master (not the bitcoinj fork we use)
So, bitcoinj master can be built using jdk 8, 9, 10 or 11.
Nobody confirmed yet that without the hack, bitcoinj does not reproduce the bug on windows.
Even if someones confirm she is running bitcoinj on windows without problems, that does not mean it will run on every windows environment.

The feature that has the windows problem is marked to be refactored in bitcoinj
https://github.com/bitcoinj/bitcoinj/blob/master/core/src/main/java/org/bitcoinj/store/SPVBlockStore.java#L33

How to solve it

Current solution

• Devs install both jdk8 and jdk10. Tell intellij to use jdk8 for bitcoinj and jdk10 for bisq. Use a bash script to switch jdk versions when the dev needs to run maven/gradle on the command line.
  • Pros
    • Oscar has the "hack" to switch jdks set up, so it is not a big issue for him to use this "hack".
  • Cons
    • Other devs coding both bisq and bitcoinj need to be aware of this hack and adapt it to their OS if they don't run on Mac.

Other possible solutions

• cherry pick bitcoinj@1700ec0 to bisq’s bitcoinj fork.
  • Pros
    • Easy to implement.
    • Devs coding both bisq and bitcoinj don't have to switch jdks - jdk 10 is ok for both projects.
  • Cons
    • Risk of bisq users having problems when running on windows (Risk could be partially reduced if we test on Windows).
• Devs cherry pick bitcoinj@1700ec0 fix locally. Devs use git stash and other git hacks to avoid pushing that change.
  • Pros
    • Easy to implement.
    • Devs coding both bisq and bitcoinj don't have to switch jdks - jdk 10 is ok for both projects.
  • Cons
    • It is a nuisance to make sure some local changes are not included every time you commit.
• Invest some time to refactor the bitcoinj feature that has the windows problem.
  • Pros
    • Solve the problem for good.
    • Devs coding both bisq and bitcoinj don't have to switch jdks - jdk 10 is ok for both projects.
  • Cons
    • May require a considerable amount of time to do the refactor.

[oscarguindzberg]

This is how I managed to run several jdks on mac:

• Install jdks using brew
• Use jenv to switch between jdks
• jenv does not work perfect, it needs extra hacking
• I wrote setjdk.sh

echo "Seting jdk $1..."

standardFile='/Contents/Info.plist'
bakFile='/Contents/Info.plist.bak'

openjdk8dir="/Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk"
openjdk10dir="/Library/Java/JavaVirtualMachines/adoptopenjdk-10.jdk"
oraclejdk8dir="/Library/Java/JavaVirtualMachines/jdk1.8.0_192.jdk"

openjdk8jenv="openjdk64-1.8.0.192"
openjdk10jenv="openjdk64-10.0.2"
oraclejdk8jenv="oracle64-1.8.0.192"


if [ "$1" == "oraclejdk8" ]; then
	jenv global $oraclejdk8jenv
	if [ -f "$openjdk8dir$standardFile" ]; then
	   sudo mv "$openjdk8dir$standardFile" "$openjdk8dir$bakFile"
	fi
	if [ -f "$openjdk10dir$standardFile" ]; then
	   sudo mv "$openjdk10dir$standardFile" "$openjdk10dir$bakFile"
	fi  
	if [ -f "$oraclejdk8dir$bakFile" ]; then
	   sudo mv "$oraclejdk8dir$bakFile" "$oraclejdk8dir$standardFile"
	fi  
fi
if [ "$1" == "openjdk8" ]; then
	jenv global $openjdk8jenv
	if [ -f "$openjdk8dir$bakFile" ]; then
	   sudo mv "$openjdk8dir$bakFile" "$openjdk8dir$standardFile"
	fi  
	if [ -f "$openjdk10dir$standardFile" ]; then
	   sudo mv "$openjdk10dir$standardFile" "$openjdk10dir$bakFile"
	fi  
	if [ -f "$oraclejdk8dir$standardFile" ]; then
	   sudo mv "$oraclejdk8dir$standardFile" "$oraclejdk8dir$bakFile"
	fi  
fi
if [ "$1" == "openjdk10" ]; then
	jenv global $openjdk10jenv
	if [ -f "$openjdk8dir$standardFile" ]; then
	   sudo mv "$openjdk8dir$standardFile" "$openjdk8dir$bakFile"
	fi
	if [ -f "$openjdk10dir$bakFile" ]; then
	   sudo mv "$openjdk10dir$bakFile" "$openjdk10dir$standardFile"
	fi  
	if [ -f "$oraclejdk8dir$standardFile" ]; then
	   sudo mv "$oraclejdk8dir$standardFile" "$oraclejdk8dir$bakFile"
	fi  
fi
echo "done."

set -o xtrace
java -version
jenv version
/usr/libexec/java_home

• I run setjdk.sh oraclejdk8 and setjdk.sh openjdk10 to switch between jdks

[ManfredKarrer]

The bitcoinj/bitcoinj@1700ec0 commit removes it but has no replacement. Is that safe?

[oscarguindzberg]

@ManfredKarrer bitcoinj/bitcoinj@1700ec0 has the risk of bisq users having problems when running on windows (Risk could be partially reduced if we test on Windows).

I updated my original post to include pros and cons of every alternative.

[oscarguindzberg]

• I created a windows box on aws to test stuff on it.
• The workaround of building bitcoinj with jdk8 and bisq with jdk10 works fine at build time. At runtime, everything is run with java 10, so there is still a problem.
• Opening and and closing the Bisq app on windows works smoothly.
  • My guess: During "bitcoinj shutdown", NoSuchMethodError is thrown when sun.misc.Cleaner code is going to be executed by WindowsMMapHack but since WalletConfig.shutdown() has a “catch Throwable” that does not do a log, nothing is printed on the log. Since the app closes, maybe locks are cleaned up and the problem is not exposed to the user.
• Bisq restore from seed on windows fails.
  • My guess: NoSuchMethodError is thrown and not logged (same thing that happens when Bisq is closed). Then when a “new bitcoinj” is created, we get an exception.
  • The UI informs the user an error has ocurred. The log prints:

Dec-26 19:26:50.393 [JavaFX Application Thread] INFO  o.b.c.MnemonicCode: PBKDF2 took 14.38 ms
Dec-26 19:26:50.632 [ STOPPING] INFO  o.b.c.PeerGroup: Awaiting PeerGroup shutdown ...
Dec-26 19:26:50.633 [PeerGroup Thread] INFO  o.b.c.PeerGroup: Stopping ...
Dec-26 19:26:51.063 [PeerGroup Thread] INFO  o.b.c.PeerGroup: Stopped.
Dec-26 19:26:51.615 [ STOPPING] INFO  o.b.s.SPVBlockStore: Windows mmap hack: Forcing buffer cleaning
Dec-26 19:26:51.766 [RestoreBTCWallet-%d] INFO  b.c.b.s.WalletsSetup: Socks5Proxy for bitcoinj: socks5Proxy=127.0.0.1:50495     Version 5
Dec-26 19:26:51.775 [RestoreBTCWallet-%d] INFO  o.b.c.Context: Creating bitcoinj 0.14.4.11 context.
Dec-26 19:26:51.777 [RestoreBTCWallet-%d] INFO  b.c.b.n.BtcNetworkConfig: You connect with peerAddresses: [[r3dsojfhwcm7x7p6.onion]:8333, [vlf5i3grro3wux24.onion]:8333, [fz6nsij6jiyuwlsc.onion]:8333, [c6ac4jdfyeiakex2.onion]:8333, [sjyzmwwu6diiit3r.onion]:8333, [3xucqntxp5ddoaz5.onion]:8333, [lgkvbvro67jomosw.onion]:8333, [4jyh6llqj264oggs.onion]:8333, [mxdtrjhe2yfsx3pg.onion]:8333, [3r44ddzjitznyahw.onion]:8333, [i3a5xtzfm4xwtybd.onion]:8333, [z33nukt7ngik3cpe.onion]:8333, [sslnjjhnmwllysv4.onion]:8333]
Dec-26 19:26:51.785 [ STARTING] INFO  b.c.b.s.WalletConfig: Wallet directory: C:\Users\Administrator\AppData\Roaming\Bisq\btc_mainnet\wallet
Dec-26 19:26:51.788 [ STARTING] INFO  b.c.b.s.WalletConfig: Renaming old wallet file C:\Users\Administrator\AppData\Roaming\Bisq\btc_mainnet\wallet\bisq_BTC.wallet to C:\Users\Administrator\AppData\Roaming\Bisq\btc_mainnet\wallet\Backup 1 for bisq_BTC.wallet
Dec-26 19:26:51.827 [ STARTING] INFO  o.b.w.DeterministicKeyChain: 133 keys needed for M/44H/0H/0H/0 = 0 issued + 100 lookahead size + 33 lookahead threshold - 0 num children
Dec-26 19:26:51.878 [ STARTING] INFO  o.b.w.DeterministicKeyChain: Took 46.38 ms
Dec-26 19:26:51.878 [ STARTING] INFO  o.b.w.DeterministicKeyChain: 133 keys needed for M/44H/0H/0H/1 = 0 issued + 100 lookahead size + 33 lookahead threshold - 0 num children
Dec-26 19:26:51.918 [ STARTING] INFO  o.b.w.DeterministicKeyChain: Took 39.75 ms
Dec-26 19:26:52.012 [ STARTING] INFO  b.c.b.s.WalletConfig: Renaming old wallet file C:\Users\Administrator\AppData\Roaming\Bisq\btc_mainnet\wallet\bisq_BSQ.wallet to C:\Users\Administrator\AppData\Roaming\Bisq\btc_mainnet\wallet\Backup 1 for bisq_BSQ.wallet
Dec-26 19:26:52.013 [ STARTING] INFO  o.b.w.DeterministicKeyChain: 133 keys needed for M/44H/142H/0H/0 = 0 issued + 100 lookahead size + 33 lookahead threshold - 0 num children
Dec-26 19:26:52.034 [ STARTING] INFO  o.b.w.DeterministicKeyChain: Took 19.19 ms
Dec-26 19:26:52.034 [ STARTING] INFO  o.b.w.DeterministicKeyChain: 133 keys needed for M/44H/142H/0H/1 = 0 issued + 100 lookahead size + 33 lookahead threshold - 0 num children
Dec-26 19:26:52.063 [ STARTING] INFO  o.b.w.DeterministicKeyChain: Took 28.23 ms
Dec-26 19:26:52.090 [JavaFX Application Thread] ERROR b.c.b.s.WalletsSetup: Service failure from state: STARTING; failure={} java.io.IOException: org.bitcoinj.store.BlockStoreException: java.nio.channels.OverlappingFileLockException
        at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:494)
        at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
        at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
        at java.base/java.lang.Thread.run(Thread.java:844)
Caused by: org.bitcoinj.store.BlockStoreException: java.nio.channels.OverlappingFileLockException
        at org.bitcoinj.store.SPVBlockStore.<init>(SPVBlockStore.java:133)
        at bisq.core.btc.setup.WalletConfig.provideBlockStore(WalletConfig.java:331)
        at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:406)
        ... 3 common frames omitted
Caused by: java.nio.channels.OverlappingFileLockException: null
        at java.base/sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
        at java.base/sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
        at java.base/sun.nio.ch.FileChannelImpl.tryLock(FileChannelImpl.java:1191)
        at java.base/java.nio.channels.FileChannel.tryLock(FileChannel.java:1160)
        at org.bitcoinj.store.SPVBlockStore.<init>(SPVBlockStore.java:106)
        ... 5 common frames omitted
Dec-26 19:26:52.097 [JavaFX Application Thread] ERROR b.d.u.GUIUtil: java.io.IOException: org.bitcoinj.store.BlockStoreException: java.nio.channels.OverlappingFileLockException

• I isolated and reproduced that behavior:

import sun.misc.Cleaner;
import sun.nio.ch.DirectBuffer;

import java.io.File;
import java.io.RandomAccessFile;
import java.nio.MappedByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;

public class WindowsTest2 {
    public static void main(String[] argv) throws Exception {
        File myfile = new File("myfile.txt");
        boolean exists = myfile.exists();
        RandomAccessFile randomAccessFile = new RandomAccessFile(myfile, "rw");
        long fileSize = getFileSize();
        if (!exists) {
            //log.info("Creating new SPV block chain file " + file);
            randomAccessFile.setLength(fileSize);
        } else if (randomAccessFile.length() != fileSize) {
            throw new RuntimeException("File size on disk does not match expected size: " +
                    randomAccessFile.length() + " vs " + fileSize);
        }

        FileChannel channel = randomAccessFile.getChannel();
        FileLock fileLock = channel.tryLock();

        MappedByteBuffer buffer = channel.map(FileChannel.MapMode.READ_WRITE, 0, fileSize);
        buffer.put(new byte[] {65,66,67});
        System.out.println("qqqq");
        buffer.force();
        System.out.println("wwww");
        if (System.getProperty("os.name").toLowerCase().contains("win")) {
            System.out.println("eeee");
            Cleaner cleaner = ((DirectBuffer) buffer).cleaner();
            System.out.println("rrr");
            if (cleaner != null) {
                System.out.println("ttt");
                cleaner.clean();
                System.out.println("yyy");
            }
            System.out.println("uuu");
        }
        System.out.println("iiii");
        buffer = null;  // Allow it to be GCd and the underlying file mapping to go away.
        System.out.println("oooo");
        randomAccessFile.close();
        System.out.println("pppp");
    }

    public final static int getFileSize() {
        return 3;
    }
}

• Running WindowsTest2 using jdk10 on windows, the log prints:

qqqq
wwww
eeee
Exception in thread "main" java.lang.NoSuchMethodError: sun.nio.ch.DirectBuffer.cleaner()Lsun/misc/Cleaner;
at WindowsTest2.main(WindowsTest2.java:34)

• I ran on windows the unit tests (bitcoinj/bitcoinj@dbc4c35) Andreas wrote on bitcoinj master upstream to make sure removing the windows hack works fine and they passed (that does mean it will pass on every windows environment)

So...

• Moving out of jdk8 for bitcoinj seems to be the way to go to solve this problem.
• If we keep developing, testing and building bitcoinj using jdk8 and run it with java 10 we might find other problems like this one.
• Then we have 2 possible solutions:
  • Add a copy of the sun.* classes to bitcoinj
  • Remove the windows hack and so the need to use sun.* (ie cherry pick what was done on bitcoinj master upstream).
• I will think a bit more about the 2 solutions and will share my conclusions here.

[oscarguindzberg]

Here are the news:

• Remove the windows hack and so the need to use sun.* (ie cherry pick what was done on bitcoinj master upstream).

  • I tested that and it fails. I get:

  •    java.io.IOException: Failed to delete chain file in preparation for restore.
       at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:417)
       at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
       at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
       at java.base/java.lang.Thread.run(Thread.java:844)```
    

  • Apparently the windows hack is still needed. I wrote a unit test to prove the windows hack is still needed and posted a comment on the upstream issue Cannot find symbol 'sun.misc.Cleaner' on MacOS/Java 9 bitcoinj/bitcoinj#1477 (comment)

• use openjdk10 for bitcoinj

  • I tried it and I found a lot of problems and had to make several "hacks" to make it work...
  • target 1.6 conflicted with code added to bitcoinj by bisq.
    • PeerAddress uses InetSocketAddress.getHostString() which is anotated as @SInCE 1.7.
    • I don't know why this works when building using oraclejdk8.
    • I had to update core/pom.xml and update maven-compiler-plugin source and target to 1.7
  • intellij can not find most or all javafx.* packages so it does not compile
    • I deleted wallettemplate project
  • maven javadoc plugin fails, so I can not build the jar
    • Error fetching URL: http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/
    • core/pom.xml : I had to remove the reference to http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/

• Then I moved to WindowsMMapHack

  • I tried to add a copy of the sun.* classes to bitcoinj: I got an error on intellij telling me I can not add sun.* classes because they were part of the java.base module.
  • So I decided to update WindowsMMapHack instead: jdk.internal.ref.Cleaner cleaner = ((sun.nio.ch.DirectBuffer) buffer).cleaner();
  • It compiles using maven and does not shows red marks on intellij.
  • But building the project on intellij still fails... it complains it can not find jdk.internal.ref nor sun.nio.ch on the build log.
  • Since I could build it with maven, I moved on.
  • I tried that with bisq and I get the same error I used to get before the fix: OverlappingFileLockException

So...

What to do next?

• Find out the exact classes that openjdk10 uses in WindowsMMapHack and try to tune it to make it work.
• Wait for responses to Cannot find symbol 'sun.misc.Cleaner' on MacOS/Java 9 bitcoinj/bitcoinj#1477 (comment)

[ManfredKarrer]

@oscarguindzberg Can you add detail logs to see why the wallet restore on Windows fails.

[oscarguindzberg]

It's posted above #17 (comment)

[ManfredKarrer]

I meant to see if the sun.* classes are really loaded or if there is a ClassNotFound exception swallowed. I though you said that there is some exception handling without logs...

[oscarguindzberg]

Fixed by #22 and #23