Upgrade JFoenix to v9.0.10 #4443
Conversation
This commit & PR upgrades JFoenix from v9.0.6 to v9.0.10, to avoid an NPE thrown when registering a DisputeAgent in an arbitartor (regtest) desktop's account view. The JFoenix com.jfoenix.adapters.ReflectionHelper class has a getField method that silently swallows a Throwable and returns null. After clicking ALT-D or ALT-N in the an arbitrator's desktop -> accounts view (to register dispute agents) a private field cannot be accessed via reflection, and bisq.desktop.components.JFXTextFieldSkinBisqStyle#updateTextPos() throws an NPE. The cause of the NPE is due to a failure to create the textNode value in the JFXTextFieldSkinBisqStyle constructor: textNode = ReflectionHelper.getFieldContent(TextFieldSkin.class, this, "textNode"); If this happens,the UI becomes unusable -- many views are blank.
|
Does the update solve the issue? It might be that its some security policy violation if you start the app from another process. If so I expect there might follow more issues. I don't know the details of your setup though. |
Yes, the upgrade does solve the issue. Why it does is something I have not yet learned from examining JFoenix src and JFoenix commit history. There are also problems stepping through src in the debugger below Not being able to find the answer directly from the JFoenix src made me fall back to others' reports. There are statements about JFoenix not being compatible with JDK 11+: an open JFoenix PR, and this stackoverflow post describing the same problem I see (using JDK 11). JFoenix Issue 995 shows others seeing the same bug, with some upgrading and passing jvm JFoenix is still active, but not very. I think Bisq will be forced to upgrade this dependency for JDK14, but JFoenix might be gradually removed in stages, first by reducing the dependency on reflection in our cloned JFoenix code. There is an example of how to do some of that in PR 1110's single commit. |
|
Thanks for the details! We should try to get rid of that library... utACK |
|
@ghubstan I'll revert this PR as I found a couple of issues during quick smoke testing. They should be fixable, but I don't have time to do it before v1.3.8. Could you please re-open this PR after the release branch for v1.3.8 is created?
|
I will |
|
Note: any jfoenix upgrade needs to be tested on JDK 10 and 11 (and 12+?). |
As long as we need to stick to JDK 10 to create the release binaries, unfortunately yes. |
ghost
mentioned this pull request
This PR upgrades
JFoenixfrom v9.0.6 to v9.0.10, to avoid an NPE thrown when registering a dispute agent in an arbitrator (regtest) desktop's account view.The JFoenix
com.jfoenix.adapters.ReflectionHelperclass has agetFieldContentmethod that silently swallows a Throwable and returns null. After clickingALT-DorALT-Nin the an arbitrator's desktop -> accounts view (register dispute agents) a private field cannot be accessed via reflection, andbisq.desktop.components.JFXTextFieldSkinBisqStyle#updateTextPosthrows an NPE.The NPE follows the failure to create a
textNodeinstance using reflection, in theJFXTextFieldSkinBisqStyleconstructor:textNode = ReflectionHelper.getFieldContent(TextFieldSkin.class, this, "textNode");If this happens, the UI becomes unusable -- many views are blank.
This problem has not been reproduced when running the
bisq-desktopcmd below from a Linux (Ubuntu 20) bash shell:$ ./bisq-desktop --appName=bisq-BTC_REGTEST_Arb_dao --appDataDir=<project-path>/apitest/build/resources/main/bisq-BTC_REGTEST_Arb_dao --nodePort=4444 --rpcBlockNotificationPort=5121 --rpcUser=apitest --rpcPassword=apitest --rpcPort=19443 --daoActivated=true --fullDaoNode=true --seedNodes=localhost:2002 --baseCurrencyNetwork=BTC_REGTEST --useDevPrivilegeKeys=true --useLocalhostForP2P=true --genesisBlockHeight=111 --genesisTxId=30af0050040befd8af25068cc697e418e09c2d8ebd8d411d2240591b9ec203cfThe same bisq-desktop cmd run by
:apitest(using a javaProcessBuilder) reproduces the NPE every time. We need to be able to register dispute agents from an arbitration node started by any API test case.