Menu

#4118 BeanShell error: module java.base does not "opens java.lang" to unnamed module

normal bug
closed-fixed
macro (3)
5
2023-10-31
2022-04-22
blurredd
No

I see the following BeanShell error when running a macro that appends to a StringBuilder:

Sourced file: C:\Users\User\AppData\Roaming\jEdit\macros\StringBuilderTest.bsh unknown error: Unable to make public java.lang.AbstractStringBuilder java.lang.AbstractStringBuilder.append(java.lang.String) accessible: module java.base does not "opens java.lang" to unnamed module @7c75222b : at Line: 2 :
 in file: C:\Users\User\AppData\Roaming\jEdit\macros\StringBuilderTest.bsh
 : sb .append ( "test" ) 

    at org.gjt.sp.jedit.bsh.Interpreter.eval(Interpreter.java:696)
    at org.gjt.sp.jedit.BeanShell._runScript(BeanShell.java:343)
    at org.gjt.sp.jedit.BeanShell._runScript(BeanShell.java:291)
    at org.gjt.sp.jedit.BeanShell.runScript(BeanShell.java:217)
    at org.gjt.sp.jedit.Macros$BeanShellHandler.runMacro(Macros.java:1108)
    at org.gjt.sp.jedit.Macros$Macro.invoke(Macros.java:530)
    at org.gjt.sp.jedit.gui.InputHandler.invokeAction(InputHandler.java:343)
    at org.gjt.sp.jedit.jEdit$3.invokeAction(jEdit.java:3417)
    at org.gjt.sp.jedit.jEdit$3.invokeAction(jEdit.java:3399)
    at org.gjt.sp.jedit.EditAction$Wrapper.actionPerformed(EditAction.java:225)
    at java.desktop/javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1972)
...

StringBuilderTest.bsh:

StringBuilder sb = new StringBuilder():
sb.append("test");

I'm using jEdit 5.6.0, java version 17.0.2 on Windows 10 Enterprise.

Discussion

  • Robert Schwenn

    Robert Schwenn - 2022-04-23

    I run into this issue, too. As I understood, that's a usual issue with Java 16 and later, when code isn't that usual (hacking-like). In former versions of Java such code should have raised warnings, but Java 17 raises exceptions.

    I.e. the snippet "TestHashMap.size" used to work in Beanshell until Java 11, although "size" isn't a field but a method. Now with Java 17 an exception is raised (similar to the above) and the code has to be changed to "TestHashMap.size()".

    Since Your example snippet seems to be accurate Java, I argue it could be a Beanshell problem and leads me to the question:

    Is BeanShell compatible with Java 17?

     
  • Dale Anson

    Dale Anson - 2022-04-23

    Well, it (your StringBuilderTest macro) works on my machine :) I'm sure the reason is that last year, I upgraded my local installation of jEdit to use Beanshell 2.1. I don't recall off the top of my head what version is currently included with jEdit (1.2 maybe?), but obviously, it doesn't quite work with Java 17. For the record, I actually tested with OpenJDK 18 as I don't have Java 17 installed.

    So I have the fix for this, but there are other issued involved, which is why I haven't checked in my beanshell changes. Beanshell is licensed under Apache 2.0, jEdit is currently licensed under GPL 2.0, and these two licenses aren't compatible. What needs to happen is for some GPL 3.0 code to be checked in, which automatically bumps all the jEdit code to GPL 3.0 and magically Apache licensed code is compatible and Beanshell can be upgraded to the latest version.

     
  • senon

    senon - 2022-05-06

    I solved it by rewriting my favorite macro code without using StringBuilder (quite simple using the standard string concat).

     
  • Robert Schwenn

    Robert Schwenn - 2023-10-06

    OK, let's restart with this issue: My system is:
    jEdit 5.6 (virgin install and settings)
    Adoptium jdk-20.0.2+9-jre
    Windows 11 Pro 22H2

    Steps to reproduce:

    1. Startup jEdit
    2. Run StringBuilderTest.bsh => works fine.
    3. Run one of these bundled macros:
      • Editing/Mode_Switcher
      • Properties/Create_Plugin_Announcement
      • Text/Insert_Tag
      • Misc/Debug_BufferSets
    4. Run StringBuilderTest.bsh => Now an error raises as described above

    The error will raise with Java 17 and later, but not with Java 11 . I have no clue where the problem is. But I hope, someone can reproduce this now an will have an idea.

     
  • Matthieu Casanova

    Hey Robert,
    can you try to add --add-opens java.desktop/java.awt=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED to the java args in your command line ?
    I agree even if it works it is not ideal.

     
  • Robert Schwenn

    Robert Schwenn - 2023-10-08

    Yes, with these java command line args the StringBuilderTest.bsh runs fine (in my step 5).

     
  • Matthieu Casanova

    Ok, maybe if we start using modules in jEdit we could fix that, however to do that we have to remove the jsr305 lib as it was never confirmed it uses packages of JRE which is forbidden

     
  • Björn Kautler

    Björn Kautler - 2023-10-13

    Besides that jsr305 would not be any problem as I wrote on the mailing list, how do you think would making jEdit a JPMS module change anything? That would not magically make BeanShell be able to do that reflection. You would still need the --add-opens either way. And you can also use the --add-opens fine without making jEdit a JPMS module. As we build the jedit.jar as executable jar and only start it like that, we can even put those as Add-Opens attribute to the manifest of jedit.jar and it would work, without jEdit being a JPMS module.

    But the bigger problem probably is, that those two are not exhaustive. They might fix this specific BeanShell snippet. But there are probably others? Potentially ever package in the JDK? I doubt we should or could add Add-Opens for the whole JDK world.

     
  • Björn Kautler

    Björn Kautler - 2023-10-13

    The point why the snippet first works and then not anymore, is the usage of classes in a macro suddenly changes configuration.

    By default Capabilities#accessibility is false.
    This means BeanShell does not try to do setAccessible and alike.
    There is exactly one place where this is set to true, which is when a class (named or anonymous) is found in a BeanShell code that is executed.
    The current implementation in ClassGeneratorImpl#generateClassImpl has the comment

    // Scripting classes currently requires accessibility
    // This can be eliminated with a bit more work.

    and then calls Capabilities.setAccessibility( true );.
    This call succeeds if the classes AccessibleObject and ReflectManagerImpl are found and the security manager does not object to the call String.class.getDeclaredMethods(); which is meant as basic reflection check call.

    We could easily add a check whether an actual isAccessible call on some private field or method succeeds, or with updating to Java 17 or 21 we could just hard-code the value to false or remove the ReflectManagerImpl class which should have the same effect.

    On the one hand this would maybe not be bad, as it adds consistency to the behavior.
    Currently, things work one way unless some BeanShell snippet with a class definition is invoked and then suddenly start to behave differently, which could mean they start to fail while worked before, start to work while failed before or continue working but change behavior.

    On the other hand, only that change would mean that no BeanShell snippets that define classes are supported anymore and things that maybe worked before after executing such a snippet might not work anymore. The latter is maybe not so tragic, but I'm not sure how bad it would be to not support classes anymore.

    On the other hand, someone could have a look at what the comment means with "This can be eliminated with a bit more work." and maybe do that more work ourselves, then it might be better acceptable to disable the accessibility capability permanently.

     
  • Björn Kautler

    Björn Kautler - 2023-10-14

    Actually, we might be more lucky than we should be. :-D
    You can easily reproduce with this snippet:

    class Foo { }
    StringBuilder sb = new StringBuilder();
    sb.append("test");
    

    The good thing is, I felt brave and just commented out the Capabilities.setAccessibility( true ); call.
    And what should I say, it worked. :-)
    So maybe that comment is not up-to-date anymore actually.

    I wouldn't do this change now shortly before the 5.7.0 release,
    but after that, we can maybe just do it and see how it works out. :-)

     
  • Björn Kautler

    Björn Kautler - 2023-10-14

    I had a quick look through the code, and I don't think something should really break by doing this, except accessing inaccessible fields that worked before after a class was defined somewhere. I'll sleep it over and maybe we can even put it in 5.7.0.

     
  • Björn Kautler

    Björn Kautler - 2023-10-14

    Ok, I slept over it and looked through the code again.
    I feel confident enough to do it still for 5.7.0, so this should hopefully be fixed now.

     
  • Björn Kautler

    Björn Kautler - 2023-10-14
    • status: open --> closed-fixed
    • assigned_to: Björn Kautler
    • Group: Regressive (new to devel) --> normal bug
     
  • Dale Anson

    Dale Anson - 2023-10-31

    Just an FYI, this is fixed in beanshell 2.1.1, which requires moving jedit to GPL 3 before we can use it.

     

Log in to post a comment.