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

getsockopt crashing with 9.2.18.0 and JRUBY_OPTS=--dev flag #6714

Closed
gsouzab opened this issue Jun 11, 2021 · 11 comments
Closed

getsockopt crashing with 9.2.18.0 and JRUBY_OPTS=--dev flag #6714

gsouzab opened this issue Jun 11, 2021 · 11 comments

Comments

@gsouzab
Copy link

gsouzab commented Jun 11, 2021

Hey all 👋
After upgrading to the latest 9.2.18.0 we started experiencing this weird error. It happens only when using the JRUBY_OPTS="--dev" flag. Let me know if you need any extra details to help with the investigation.

Environment Information

  • JRuby 9.2.18.0
  • JRUBY_OPTS="--dev"

Expected Behavior
getsockopt works

Actual Behavior
When using JRUBY_OPTS=--dev getsockopt crashes with the following stacktrace

Traceback (most recent call last): 16: from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:85) 15: from org.jruby.RubyKernel$INVOKER$s$0$3$eval.call(RubyKernel$INVOKER$s$0$3$eval.gen) 14: from org.jruby.RubyKernel.eval(RubyKernel.java:1048) 13: from org.jruby.RubyKernel.evalCommon(RubyKernel.java:1086) 12: from org.jruby.ir.interpreter.Interpreter.evalWithBinding(Interpreter.java:182) 11: from org.jruby.ir.interpreter.Interpreter.evalCommon(Interpreter.java:158) 10: from org.jruby.ir.interpreter.Interpreter.INTERPRET_EVAL(Interpreter.java:106) 9: from org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72) 8: from org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:325) 7: from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:206) 6: from org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:397) 5: from org.jruby.ext.socket.RubyBasicSocket$INVOKER$i$2$0$getsockopt.call(RubyBasicSocket$INVOKER$i$2$0$getsockopt.gen) 4: from org.jruby.ext.socket.RubyBasicSocket.getsockopt(RubyBasicSocket.java:430) 3: from com.sun.proxy.$Proxy30.getsockoptInt(Unknown Source) 2: from jnr.ffi.provider.NativeInvocationHandler.invoke(NativeInvocationHandler.java:47) 1: from jnr.ffi.provider.jffi.DefaultInvokerFactory$FunctionNotFoundInvoker.invoke(DefaultInvokerFactory.java:465) Java::JavaLang::UnsatisfiedLinkError (native method 'getsockoptInt' not found for method public default int org.jruby.ext.socket.RubyBasicSocket$LibC.getsockoptInt(int,int,int,java.nio.ByteBuffer,jnr.ffi.byref.IntByReference))

How to reproduce
Using the latest JRuby image run the following snippet

require "socket"
puts(JRUBY_VERSION) # to make sure it's 9.2.18.0
s = TCPServer.new(2560)
s.getsockopt(Socket::IPPROTO_TCP, Socket::TCP_INFO) # this is where it crashes

Using irb:

docker run -it jruby:9.2.18.0 bin/bash
JRUBY_OPTS="--dev" irb
require "socket"
puts(JRUBY_VERSION) # to make sure it's 9.2.18.0
s = TCPServer.new(2560)
s.getsockopt(Socket::IPPROTO_TCP, Socket::TCP_INFO) # this is where it crashes

Watch it crash.

@ahorek
Copy link
Contributor

ahorek commented Jun 11, 2021

auch, unfortunately, we need native support which is disabled in the dev mode.

but UnsatisfiedLinkError is something that shouldn't happen, what about ENOPROTOOPT ? what do you think @headius ?

@ThomasKoppensteiner
Copy link

Is this related to the backport done for #6541 ?

@ahorek
Copy link
Contributor

ahorek commented Jun 12, 2021

@ThomasKoppensteiner yes. Unfortunately, we didn't catch it earlier.
for now, please use JRUBY_OPTS="--dev -J-Djnr.ffi.asm.enabled=true" as a workaround

I would expect that the dev mode shouldn't disable features and change the behavior, let's discuss with @headius about possible solutions

@ahorek ahorek added this to the JRuby 9.2.19.0 milestone Jun 12, 2021
@headius
Copy link
Member

headius commented Jun 14, 2021

@ahorek I wouldn't consider this to be a behavioral feature, as the jnr-ffi code generation is supposed to be identical in behavior but somewhat faster.

That said, I discovered some time ago in jnr/jnr-ffi#201 that the non-ASM mode had gotten stale and introduced some test failures. I added a test run that would use the non-ASM mode and got it passing, but it seems there's further work required.

@headius
Copy link
Member

headius commented Jun 14, 2021

I think I see the issue here. The interface we use to bind the get/setsockopt functions has two additional default interface methods, which should not be bound by the jnr-ffi subsystem. They are intended as utility methods which call the appropriate native interface.

@ahorek
Copy link
Contributor

ahorek commented Jun 14, 2021

thanks for your input @headius !

I can confirm that #6723 fixes the issue in --dev mode

do you think we should catch UnsatisfiedLinkError in case the library cannot be loaded for some other reason, just to be sure?

@headius
Copy link
Member

headius commented Jun 14, 2021

@ahorek We could but that would mean wrapping every native call with exception handling. It might be better for us to extend jnr-ffi to accept an error handler, similar to jnr-posix, which could raise a better error.

@headius
Copy link
Member

headius commented Jun 14, 2021

I have filed jnr/jnr-ffi#249 for this problem and pushed a prototype fix in jnr/jnr-ffi#250, but I am not sure the solution is worth the hassle.

In JRuby, the following patch will take advantage of the changes in #250 and allow the interface's default methods to invoke properly:

diff --git a/core/src/main/java/org/jruby/ext/socket/RubyBasicSocket.java b/core/src/main/java/org/jruby/ext/socket/RubyBasicSocket.java
index efcf5193a9..35a5a7267c 100644
--- a/core/src/main/java/org/jruby/ext/socket/RubyBasicSocket.java
+++ b/core/src/main/java/org/jruby/ext/socket/RubyBasicSocket.java
@@ -65,6 +65,7 @@ import org.jruby.util.io.OpenFile;
 import org.jruby.util.io.Sockaddr;
 
 import java.io.IOException;
+import java.lang.invoke.MethodHandles;
 import java.net.Inet6Address;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -536,6 +537,7 @@ public class RubyBasicSocket extends RubyIO {
         int F_GETFL = jnr.constants.platform.Fcntl.F_GETFL.intValue();
         int F_SETFL = jnr.constants.platform.Fcntl.F_SETFL.intValue();
         int O_NONBLOCK = jnr.constants.platform.OpenFlags.O_NONBLOCK.intValue();
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
 
         int getsockopt(int s, int level, int optname, @Out ByteBuffer optval, @Out IntByReference optlen);
         int setsockopt(int s, int level, int optname, @In ByteBuffer optval, int optlen);
@@ -555,7 +557,7 @@ public class RubyBasicSocket extends RubyIO {
     static final LibC SOCKOPT;
 
     static {
-        LibraryLoader<LibC> loader = LibraryLoader.create(LibC.class);
+        LibraryLoader<LibC> loader = LibraryLoader.create(LibC.class, LibC.lookup);
         for (String libraryName : libnames) {
             loader.library(libraryName);
         }

@headius
Copy link
Member

headius commented Jun 14, 2021

Fixed by #6273. We will debate how best to fix jnr-ffi, but short term we will just say it does not support interfaces with default methods unless you allow it to generate code.

@headius headius closed this as completed Jun 14, 2021
@Farzin3
Copy link

Farzin3 commented Aug 16, 2021

ahorek من این را یک ویژگی رفتاری نمی دانم ، زیرا نسل کد jnr-ffi از نظر رفتاری یکسان است اما تا حدودی سریعتر است.

با این اوصاف ، مدتی پیش در jnr/jnr-ffi#201 متوجه شدم که حالت غیر ASM قدیمی شده است و برخی از شکست های آزمایش را معرفی کرده است. من یک برنامه آزمایشی اضافه کردم که از حالت غیر ASM استفاده می کرد و آن را قبول کردم ، اما به نظر می رسد کار بیشتری مورد نیاز است.

@Farzin3
Copy link

Farzin3 commented Aug 16, 2021

تشخیص ضریب

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants