Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

become_java! not compatible with Java sub-class generated proxies #4274

Closed
pilhuhn opened this issue Nov 10, 2016 · 20 comments
Closed

become_java! not compatible with Java sub-class generated proxies #4274

pilhuhn opened this issue Nov 10, 2016 · 20 comments

Comments

@pilhuhn
Copy link
Contributor

pilhuhn commented Nov 10, 2016

Environment

Provide at least:

  • jruby -G -S hawkfx.rb
  • jruby -version
    jruby 9.1.6.0 (2.3.1) 2016-11-09 0150a76 Java HotSpot(TM) 64-Bit Server VM 25.111-b14 on 1.8.0_111-b14 +jit [darwin-x86_64]
  • OS/X Sierra $ uname -a
    Darwin snert 16.1.0 Darwin Kernel Version 16.1.0: Thu Oct 13 21:26:57 PDT 2016; root:xnu-3789.21.3~60/RELEASE_X86_64 x86_64

This used to work in 9.1.5 and fails in 9.1.6

/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: `<<' after local variable or literal is interpreted as binary operator
/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:158: warning: even though it seems like here document
Exception in thread "JavaFX Application Thread" org.jruby.exceptions.RaiseException: (TypeError) requested class ChartViewController was not reifiable
	at org.jruby.java.addons.ClassJavaAddons.become_java!(org/jruby/java/addons/ClassJavaAddons.java:40)
	at RUBY.new(/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-1.1.1-java/lib/jrubyfx/controller.rb:147)
	at Users.hrupp.$_dot_rvm.gems.jruby_minus_9_dot_1_dot_6_dot_0.gems.jrubyfx_minus_1_dot_1_dot_1_minus_java.lib.jrubyfx.module.build(/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-1.1.1-java/lib/jrubyfx/module.rb:92)
	at RUBY.method_missing(/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-1.1.1-java/lib/jrubyfx/dsl.rb:108)
	at RUBY.show_initial_tree(/Users/hrupp/src/hawkfx/lib/hawk_main_controller.rb:24)
	at RUBY.show_main_pane(/Users/hrupp/src/hawkfx/lib/hawk_login_controller.rb:118)
	at RUBY.login(/Users/hrupp/src/hawkfx/lib/hawk_login_controller.rb:87)
	at RUBY.handle(/Users/hrupp/.rvm/gems/jruby-9.1.6.0/gems/jrubyfx-fxmlloader-0.4.1-java/lib/fxmlloader/elts.rb:521)

The full code is at https://github.com/pilhuhn/hawkfx with the failing controller at https://github.com/pilhuhn/hawkfx/blob/master/lib/chart_view_controller.rb

@presidentbeef
Copy link

If this is due to e2027ed what was happening in 9.1.5.0 and earlier? It just failed but applications worked anyhow?

@zmoazeni
Copy link

zmoazeni commented Dec 28, 2016

Here's a simpler example I ran into while trying to play with JRuby and JavaFX (don't need to mess with any dependencies to see the error):

require "java"

class Foo < Java.javafx.application.Application
  def start(primaryStage)
    primaryStage.setTitle("Hello World!")
    btn = Java::javafx::scene::control::Button.new
    btn.setText("Say 'Hello World'")
    btn.setOnAction do
      p "btn pressed"
    end

    root = Java.javafx.scene.layout.StackPane.new
    root.getChildren().add(btn)
    primaryStage.setScene(Java::javafx::scene::Scene.new(root, 300, 250))
    primaryStage.show()
  end
end


Foo.become_java!
Java::javafx::application::Application.launch(Foo.java_class, ARGV.to_java(:string))
$ ruby hello_world.rb
TypeError: requested class Foo was not reifiable
  become_java! at org/jruby/java/addons/ClassJavaAddons.java:40
        <main> at hello_world.rb:20

I was able to work around it by compiling a java class and setting a proc on it, but it would have been nice to avoid that:

import javafx.application.Application;
import javafx.stage.Stage;

// JavaFX needs an Application to instantiate. So we define a Java class that will call Ruby
public class JavaFXBootstrap extends Application {

    public interface MyLauncher {
        public void launch(Stage primaryStage);
    }

    public static MyLauncher rubyLauncher;

    public void start(Stage primaryStage) {
        rubyLauncher.launch(primaryStage);
    }
}
Java::JavaFXBootstrap.rubyLauncher = proc {|primaryStage|
  primaryStage.setTitle("Hello World!")
  btn = Java::javafx::scene::control::Button.new
  btn.setText("Say 'Hello World'")
  btn.setOnAction do
    p "btn pressed"
  end

  root = Java.javafx.scene.layout.StackPane.new
  root.getChildren().add(btn)
  primaryStage.setScene(Java::javafx::scene::Scene.new(root, 300, 250))
  primaryStage.show()
}
Java::javafx::application::Application.launch(Java::JavaFXBootstrap.java_class, ARGV.to_java(:string))

Edit:

If this helps:

$ java -version
openjdk version "1.8.0_111"
OpenJDK Runtime Environment (build 1.8.0_111-8u111-b14-2ubuntu0.16.04.2-b14)
OpenJDK 64-Bit Server VM (build 25.111-b14, mixed mode)

$ ruby --version
jruby 9.1.6.0 (2.3.1) 2016-11-09 0150a76 OpenJDK 64-Bit Server VM 25.111-b14 on 1.8.0_111-8u111-b14-2ubuntu0.16.04.2-b14 +jit [linux-x86_64]

I'm using JFX 8u60

@kares
Copy link
Member

kares commented Dec 28, 2016

@presidentbeef yy - might have worked but there probably might have been issues (due reify failure) later.
@zmoazeni najs simplified example -> certainly helps get this fixed ... now if only I had time to look into it :)

@kares
Copy link
Member

kares commented Dec 28, 2016

OH, this one is likely the same as #4165 ... an important detail :

I missed a detail in your example: you're extending a concrete Java class. become_java! and concrete extension have never been wired up together, so I'm not at all surprised that this failed.

... have been looking into this before and I do not have a quick solution (requires considerable work on the internals of these features). so the work-around is probably the only way to get this working.
JRuby's current limitation is: if you extend a Java class do not use become_java! it won't work correctly

@kares kares changed the title "(TypeError) requested class .. was not reifiable" in 9.1.6, worked in 9.1.5 become_java! not compatible with Java sub-class generated proxies Dec 28, 2016
@pilhuhn
Copy link
Contributor Author

pilhuhn commented Dec 28, 2016 via email

@kares
Copy link
Member

kares commented Dec 28, 2016

in that case I believe it might get reverted but others might seem issues such as NPE (as in #4165)
.. maybe a middle ground would be to revert but instead of raising print a warning message //cc @headius ?

@presidentbeef
Copy link

This is a problem when using JRubyFX since it overrides new and calls become_java! https://github.com/jruby/jrubyfx/blob/c6327641a8d3f68b3f6c7b65d35cce13fa9d103e/lib/jrubyfx/controller.rb#L151-L152

@byteit101
Copy link
Member

byteit101 commented Jan 14, 2017

JRubyFX author here, and yes, that file was carefully crafted before to work well, and it was working correctly (at least in relation to JavaFX interop) from 1.7.3 - 9.1.5. This is a regression.

@pilhuhn
Copy link
Contributor Author

pilhuhn commented Mar 6, 2017

Still broken in 9.1.8.0 :-(((

@headius
Copy link
Member

headius commented Mar 6, 2017

Indeed...we had to cut off 9.1.8.0 before it got too fat. This tab is still on my list.

@presidentbeef
Copy link

Just a small poke on this - can't switch from 1.7 to 9.x until this is fixed 😭

@enebo enebo added this to the JRuby 9.2.0.0 milestone May 19, 2017
@headius
Copy link
Member

headius commented May 23, 2017

I'll take a shot at this during my next flight.

@enebo enebo modified the milestones: JRuby 9.1.10.0, JRuby 9.2.0.0 May 23, 2017
@enebo
Copy link
Member

enebo commented May 23, 2017

I think we try and understand what JRubyFX needed from become_java!. Worst case for 9.1.10.0 (out this week for sure) we just revert the raise. It would be nice if we can do something which does not reexpose the original NPE it is protecting.

@zmoazeni
Copy link

I think we try and understand what JRubyFX needed from become_java!

This is what I tracked down to the JavaFX code when I did my original digging before: https://github.com/teamfx/openjfx-8u-dev-rt/blob/8ae6126201acc37c26c2ab1b291f075c5f437168/modules/graphics/src/main/java/javafx/application/Application.java#L254-L255 (This is a linkable mirror)

JavaFX actually checks that the application class is a subclass and raises an error otherwise. That's why I ended up with my compiled JavaFXBootstrap shim in the above example.

@enebo
Copy link
Member

enebo commented May 25, 2017

@zmoazeni I believe your reduced example is different than what was originally reported and also what @presidentbeef & @pilhuhn are running into atm. Your example is showing perfectly the issue description but I think the roadblock for others is not this specifically.

What we know is that like @zmoazeni example that we have never worked in reifying this case. This was why @headius put the raise in. The reified class internally fails and is null so it should not be doing anything (that we can figure out).

So @presidentbeef and @pilhuhn are seeing issues with JRubyFX hitting this new raise in controller.rb:152 when it tries to become_java!. If I remove the raise it looks like things work in hawkfx (although there is some other error which I think may just be an FX issue involving LineChart) and JRubyFX demo works. HOWEVER, if I just delete the become_java! from controller.rb:152 then both examples also works?

At this point I am still looking for why that line in controller.rb:152 is neccesary. I believe @byteit101 that he put it there for a reason but I need to see an example of it before we can fix it.

For 9.1.10.0 (which I am putting out today) we are just going to revert this extra raise so everyone's stuff should continue to work as it did in 9.1.5.0. I still want to dig on this one as well though.

@enebo
Copy link
Member

enebo commented May 25, 2017

@presidentbeef would it be possible for you to try commenting out that line (controller.rb:152) in installed jrubyfx gem and see if you run into issues with your app? As I said, we reverted the raise for now but I want another datapoint.

@presidentbeef
Copy link

Commenting out that line appears to make no difference - all tests pass.

@enebo
Copy link
Member

enebo commented May 25, 2017

@presidentbeef ok thanks for checking on that.

@pilhuhn
Copy link
Contributor Author

pilhuhn commented Jun 15, 2017

Apparently the change that makes this fail was reverted in 9.1.11 (or earlier?) so this very issue works with 9.1.11.

@enebo
Copy link
Member

enebo commented Jun 15, 2017

@pilhuhn I got this crossed with another issue and it should have been marked against 9.1.10.0. Closing now.

@enebo enebo closed this as completed Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants