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

IO.select always runs FFI stub #2332

Closed
chrisseaton opened this issue Apr 19, 2021 · 3 comments
Closed

IO.select always runs FFI stub #2332

chrisseaton opened this issue Apr 19, 2021 · 3 comments
Assignees
Labels
Milestone

Comments

@chrisseaton
Copy link
Collaborator

Bug introduced in d997f0b.

SELECT = method(:truffleposix_select) captures the FFI lazy stub method - it's never replaced when the method is actually attached - and the stub is run every time.

Discovered with @LillianZ. She's worked around by doing the below but does this break interruptibility?

https://github.com/oracle/truffleruby/compare/master...Shopify:IO?expand=1

From 56834f4989bfae24296fab69fa9f91a983b3fedb Mon Sep 17 00:00:00 2001
From: Lillian Zhang <lillian.zhang@shopify.com>
Date: Mon, 19 Apr 2021 16:50:41 -0400
Subject: [PATCH] Avoid running attach_function_eagerly every time
 truffleposix_select is

---
 src/main/ruby/truffleruby/core/posix.rb              |  4 +---
 .../ruby/truffleruby/core/truffle/io_operations.rb   | 12 ++++++------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/main/ruby/truffleruby/core/posix.rb b/src/main/ruby/truffleruby/core/posix.rb
index ab365580a84..522d01858bb 100644
--- a/src/main/ruby/truffleruby/core/posix.rb
+++ b/src/main/ruby/truffleruby/core/posix.rb
@@ -221,7 +221,7 @@ def self.attach_function_eagerly(native_name, argument_types, return_type,
   attach_function :truffleposix_rewinddir, [:pointer], :void, LIBTRUFFLEPOSIX
   attach_function :rmdir, [:string], :int
   attach_function :seekdir, [:pointer, :long], :void
-  attach_function :truffleposix_select, [:int, :pointer, :int, :pointer, :int, :pointer, :long], :int, LIBTRUFFLEPOSIX
+  attach_function :truffleposix_select, [:int, :pointer, :int, :pointer, :int, :pointer, :long], :int, LIBTRUFFLEPOSIX, true
   attach_function :truffleposix_stat, [:string, :pointer], :int, LIBTRUFFLEPOSIX
   attach_function :truffleposix_stat_mode, [:string], :mode_t, LIBTRUFFLEPOSIX
   attach_function :truffleposix_stat_size, [:string], :long, LIBTRUFFLEPOSIX
@@ -311,8 +311,6 @@ def self.unsetenv(name)
     attach_function :dup3, [:int, :int, :int], :int
   end
 
-  SELECT = method(:truffleposix_select)
-
   def self.with_array_of_ints(ints)
     if ints.empty?
       yield Truffle::FFI::Pointer::NULL
diff --git a/src/main/ruby/truffleruby/core/truffle/io_operations.rb b/src/main/ruby/truffleruby/core/truffle/io_operations.rb
index a2120e055d4..4b959f95491 100644
--- a/src/main/ruby/truffleruby/core/truffle/io_operations.rb
+++ b/src/main/ruby/truffleruby/core/truffle/io_operations.rb
@@ -145,12 +145,12 @@ def self.select(readables, readable_ios, writables, writable_ios, errorables, er
         to_fds(errorable_ios, errorables_pointer)
 
         begin
-          primitive_result = Primitive.thread_run_blocking_nfi_system_call(Truffle::POSIX::SELECT, [
-              readables.size, readables_pointer,
-              writables.size, writables_pointer,
-              errorables.size, errorables_pointer,
-              remaining_timeout
-          ])
+          primitive_result = Truffle::POSIX.truffleposix_select(
+            readables.size, readables_pointer,
+            writables.size, writables_pointer,
+            errorables.size, errorables_pointer,
+            remaining_timeout
+          )
 
           result =
             if primitive_result < 0
@eregon
Copy link
Member

eregon commented Apr 20, 2021

Good catch!
For select() we can't use attach_function_eagerly's blocking=true because we need extra behavior to handle the timeout before retrying the call.
So I think we need to capture the method later, when the lazy stub has been resolved.

@eregon eregon added the bug label Apr 20, 2021
@eregon eregon added this to the 21.2.0 milestone Apr 20, 2021
@eregon
Copy link
Member

eregon commented Apr 20, 2021

I'm curious, how did you find out this issue? Looking at invalidations?

@chrisseaton
Copy link
Collaborator Author

It shows up in profiling.

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

No branches or pull requests

2 participants