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

Allow Monitor#interests to be nil #183

Merged
merged 6 commits into from
Mar 16, 2018
Merged

Allow Monitor#interests to be nil #183

merged 6 commits into from
Mar 16, 2018

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Feb 2, 2018

Fixes #138

@ioquatix
Copy link
Member Author

ioquatix commented Feb 2, 2018

For some reason, on my Mac (which is shite for networking anyway) the tests are quite flakey:

Failures:

  1) UDPSocket behaves like an NIO selectable selects readable objects
     Failure/Error: sock.bind("127.0.0.1", udp_port)
     
     Errno::EADDRINUSE:
       Address already in use - bind(2) for "127.0.0.1" port 23456
     Shared Example Group: "an NIO selectable" called from ./spec/nio/selectables/udp_socket_spec.rb:42
     # ./spec/nio/selectables/udp_socket_spec.rb:10:in `bind'
     # ./spec/nio/selectables/udp_socket_spec.rb:10:in `block (2 levels) in <top (required)>'
     # ./spec/support/selectable_examples.rb:7:in `block (2 levels) in <top (required)>'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:115:in `block in run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `loop'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:33:in `block (2 levels) in setup'

  2) TCPSocket behaves like an NIO selectable selects readable objects
     Failure/Error: expect(ready).to be_an Enumerable
       expected nil to be a kind of Enumerable
     Shared Example Group: "an NIO selectable" called from ./spec/nio/selectables/tcp_socket_spec.rb:69
     # ./spec/support/selectable_examples.rb:9:in `block (2 levels) in <top (required)>'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:115:in `block in run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `loop'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:33:in `block (2 levels) in setup'

  3) TCPSocket behaves like an NIO bidirectional stream keeps readiness after the selectable has been closed
     Failure/Error: expect(m.readiness).to eq(:rw)
     
       expected: :rw
            got: :w
     
       (compared using ==)
     
       Diff:
       @@ -1,2 +1,2 @@
       -:rw
       +:w
       
     Shared Example Group: "an NIO bidirectional stream" called from ./spec/nio/selectables/tcp_socket_spec.rb:71
     # ./spec/support/selectable_examples.rb:60:in `block (3 levels) in <top (required)>'
     # ./spec/support/selectable_examples.rb:59:in `select'
     # ./spec/support/selectable_examples.rb:59:in `block (2 levels) in <top (required)>'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:115:in `block in run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `loop'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:104:in `run'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # /Users/samuel/.rvm/gems/ruby-2.4.0/gems/rspec-retry-0.5.6/lib/rspec/retry.rb:33:in `block (2 levels) in setup'

Finished in 1.24 seconds (files took 0.23694 seconds to load)
104 examples, 3 failures, 1 pending

Failed examples:

rspec './spec/nio/selectables/udp_socket_spec.rb[1:1:1]' # UDPSocket behaves like an NIO selectable selects readable objects
rspec './spec/nio/selectables/tcp_socket_spec.rb[1:1:1]' # TCPSocket behaves like an NIO selectable selects readable objects
rspec './spec/nio/selectables/tcp_socket_spec.rb[1:3:2]' # TCPSocket behaves like an NIO bidirectional stream keeps readiness after the selectable has been closed

Randomized with seed 44170

@ioquatix
Copy link
Member Author

ioquatix commented Feb 2, 2018

I especially don't understand (3) above.

@tarcieri
Copy link
Contributor

@ioquatix the test suite has pretty much always been buggy on OS X, and the failures are both sporadic and very difficult to reproduce in something like a pry session

I'll get around to reviewing this soon, I hope

@ioquatix
Copy link
Member Author

Awesome - it's not actually that urgent, because I need to support backwards compatibility for a long time, but if we can merge this and then do a minor release bump I can target it from async which will simplify the code a bit.

I was also thinking about this a bit more - if the interests are set to 0, does the IO actually get scheduled? I see we add and remove it every time the interests change, but we don't actually check the interests were different or if they were 0 or not. Just wondering if it would make sense to handle these cases in nio4r or whether it's better just to leave it to libev.

@ioquatix
Copy link
Member Author

I may take another look at the test suite to see if I can isolate the issue, it's helpful to know it's a known issue. Thanks.

@ioquatix
Copy link
Member Author

@tarcieri Can you please review this and merge? It's a really simple change, but I think it's wise to have a 2nd set of eyes. If you can't do it let me know and I'll see if someone else is interested.

@tarcieri
Copy link
Contributor

Haha wow indeed, that is small. Sorry for taking so long to review it

@tarcieri tarcieri merged commit 278e5fd into master Mar 16, 2018
@tarcieri tarcieri deleted the allow-interests-nil branch March 16, 2018 01:53
@tarcieri tarcieri changed the title Allow interests nil Allow Monitor#interests to be nil Mar 16, 2018
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 17, 2018
## 2.3.0 (2018-03-15)

* [#183](socketry/nio4r#183)
  Allow `Monitor#interests` to be nil
  ([@ioquatix])
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

Successfully merging this pull request may close these issues.

2 participants