Skip to content

Commit 54c504d

Browse files
authored
Don't add IO#wait* methods when RUBY_IO_WAIT_METHODS is defined by Ruby. (#19)
* Fix return value compatibility with Ruby 2.x. * Don't add `IO#wait*` methods in Ruby 3.2+.
1 parent 35f0168 commit 54c504d

File tree

2 files changed

+103
-41
lines changed

2 files changed

+103
-41
lines changed

ext/io/wait/wait.c

Lines changed: 75 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -95,21 +95,24 @@ io_nread(VALUE io)
9595

9696
#ifdef HAVE_RB_IO_WAIT
9797
static VALUE
98-
io_wait_event(VALUE io, int event, VALUE timeout)
98+
io_wait_event(VALUE io, int event, VALUE timeout, int return_io)
9999
{
100100
VALUE result = rb_io_wait(io, RB_INT2NUM(event), timeout);
101101

102102
if (!RB_TEST(result)) {
103-
return Qnil;
103+
return Qnil;
104104
}
105105

106106
int mask = RB_NUM2INT(result);
107107

108108
if (mask & event) {
109-
return io;
109+
if (return_io)
110+
return io;
111+
else
112+
return result;
110113
}
111114
else {
112-
return Qfalse;
115+
return Qfalse;
113116
}
114117
}
115118
#endif
@@ -137,15 +140,15 @@ io_ready_p(VALUE io)
137140
if (rb_io_read_pending(fptr)) return Qtrue;
138141

139142
#ifndef HAVE_RB_IO_WAIT
140-
if (wait_for_single_fd(fptr, RB_WAITFD_IN, &tv))
141-
return Qtrue;
143+
return wait_for_single_fd(fptr, RB_WAITFD_IN, &tv) ? Qtrue : Qfalse;
142144
#else
143-
if (RTEST(io_wait_event(io, RUBY_IO_READABLE, RB_INT2NUM(0))))
144-
return Qtrue;
145+
return io_wait_event(io, RUBY_IO_READABLE, RB_INT2NUM(0), 1);
145146
#endif
146-
return Qfalse;
147147
}
148148

149+
// Ruby 3.2+ can define these methods. This macro indicates that case.
150+
#ifndef RUBY_IO_WAIT_METHODS
151+
149152
/*
150153
* call-seq:
151154
* io.wait_readable -> truthy or falsy
@@ -184,7 +187,7 @@ io_wait_readable(int argc, VALUE *argv, VALUE io)
184187
rb_check_arity(argc, 0, 1);
185188
VALUE timeout = (argc == 1 ? argv[0] : Qnil);
186189

187-
return io_wait_event(io, RUBY_IO_READABLE, timeout);
190+
return io_wait_event(io, RUBY_IO_READABLE, timeout, 1);
188191
#endif
189192
}
190193

@@ -220,7 +223,7 @@ io_wait_writable(int argc, VALUE *argv, VALUE io)
220223
rb_check_arity(argc, 0, 1);
221224
VALUE timeout = (argc == 1 ? argv[0] : Qnil);
222225

223-
return io_wait_event(io, RUBY_IO_WRITABLE, timeout);
226+
return io_wait_event(io, RUBY_IO_WRITABLE, timeout, 1);
224227
#endif
225228
}
226229

@@ -231,7 +234,8 @@ io_wait_writable(int argc, VALUE *argv, VALUE io)
231234
* io.wait_priority(timeout) -> truthy or falsy
232235
*
233236
* Waits until IO is priority and returns a truthy value or a falsy
234-
* value when times out.
237+
* value when times out. Priority data is sent and received using
238+
* the Socket::MSG_OOB flag and is typically limited to streams.
235239
*
236240
* You must require 'io/wait' to use this method.
237241
*/
@@ -248,7 +252,7 @@ io_wait_priority(int argc, VALUE *argv, VALUE io)
248252
rb_check_arity(argc, 0, 1);
249253
VALUE timeout = argc == 1 ? argv[0] : Qnil;
250254

251-
return io_wait_event(io, RUBY_IO_PRIORITY, timeout);
255+
return io_wait_event(io, RUBY_IO_PRIORITY, timeout, 1);
252256
}
253257
#endif
254258

@@ -286,10 +290,22 @@ wait_mode_sym(VALUE mode)
286290
return 0;
287291
}
288292

293+
#ifdef HAVE_RB_IO_WAIT
294+
static inline rb_io_event_t
295+
io_event_from_value(VALUE value)
296+
{
297+
int events = RB_NUM2INT(value);
298+
299+
if (events <= 0) rb_raise(rb_eArgError, "Events must be positive integer!");
300+
301+
return events;
302+
}
303+
#endif
304+
289305
/*
290306
* call-seq:
291-
* io.wait(events, timeout) -> truthy or falsy
292-
* io.wait(timeout = nil, mode = :read) -> truthy or falsy.
307+
* io.wait(events, timeout) -> event mask, false or nil
308+
* io.wait(timeout = nil, mode = :read) -> self, true, or false
293309
*
294310
* Waits until the IO becomes ready for the specified events and returns the
295311
* subset of events that become ready, or a falsy value when times out.
@@ -335,43 +351,59 @@ io_wait(int argc, VALUE *argv, VALUE io)
335351
#else
336352
VALUE timeout = Qundef;
337353
rb_io_event_t events = 0;
354+
int return_io = 0;
338355

356+
// The documented signature for this method is actually incorrect.
357+
// A single timeout is allowed in any position, and multiple symbols can be given.
358+
// Whether this is intentional or not, I don't know, and as such I consider this to
359+
// be a legacy/slow path.
339360
if (argc != 2 || (RB_SYMBOL_P(argv[0]) || RB_SYMBOL_P(argv[1]))) {
340-
for (int i = 0; i < argc; i += 1) {
341-
if (RB_SYMBOL_P(argv[i])) {
342-
events |= wait_mode_sym(argv[i]);
343-
}
344-
else if (timeout == Qundef) {
345-
rb_time_interval(timeout = argv[i]);
346-
}
347-
else {
348-
rb_raise(rb_eArgError, "timeout given more than once");
349-
}
350-
}
351-
if (timeout == Qundef) timeout = Qnil;
361+
// We'd prefer to return the actual mask, but this form would return the io itself:
362+
return_io = 1;
363+
364+
// Slow/messy path:
365+
for (int i = 0; i < argc; i += 1) {
366+
if (RB_SYMBOL_P(argv[i])) {
367+
events |= wait_mode_sym(argv[i]);
368+
}
369+
else if (timeout == Qundef) {
370+
rb_time_interval(timeout = argv[i]);
371+
}
372+
else {
373+
rb_raise(rb_eArgError, "timeout given more than once");
374+
}
375+
}
376+
377+
if (timeout == Qundef) timeout = Qnil;
378+
379+
if (events == 0) {
380+
events = RUBY_IO_READABLE;
381+
}
352382
}
353-
else /* argc == 2 */ {
354-
events = RB_NUM2UINT(argv[0]);
355-
timeout = argv[1];
356-
}
357-
358-
if (events == 0) {
359-
events = RUBY_IO_READABLE;
383+
else /* argc == 2 and neither are symbols */ {
384+
// This is the fast path:
385+
events = io_event_from_value(argv[0]);
386+
timeout = argv[1];
360387
}
361388

362389
if (events & RUBY_IO_READABLE) {
363-
rb_io_t *fptr = NULL;
364-
RB_IO_POINTER(io, fptr);
365-
366-
if (rb_io_read_pending(fptr)) {
367-
return Qtrue;
368-
}
390+
rb_io_t *fptr = NULL;
391+
RB_IO_POINTER(io, fptr);
392+
393+
if (rb_io_read_pending(fptr)) {
394+
// This was the original behaviour:
395+
if (return_io) return Qtrue;
396+
// New behaviour always returns an event mask:
397+
else return RB_INT2NUM(RUBY_IO_READABLE);
398+
}
369399
}
370400

371-
return io_wait_event(io, events, timeout);
401+
return io_wait_event(io, events, timeout, return_io);
372402
#endif
373403
}
374404

405+
#endif /* RUBY_IO_WAIT_METHODS */
406+
375407
/*
376408
* IO wait methods
377409
*/
@@ -386,11 +418,13 @@ Init_wait(void)
386418
rb_define_method(rb_cIO, "nread", io_nread, 0);
387419
rb_define_method(rb_cIO, "ready?", io_ready_p, 0);
388420

421+
#ifndef RUBY_IO_WAIT_METHODS
389422
rb_define_method(rb_cIO, "wait", io_wait, -1);
390423

391424
rb_define_method(rb_cIO, "wait_readable", io_wait_readable, -1);
392425
rb_define_method(rb_cIO, "wait_writable", io_wait_writable, -1);
393426
#ifdef HAVE_RB_IO_WAIT
394427
rb_define_method(rb_cIO, "wait_priority", io_wait_priority, -1);
395428
#endif
429+
#endif
396430
}

test/io/wait/test_io_wait.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,34 @@ def test_wait_readwrite_timeout
161161
assert_equal @w, @w.wait(0.01, :read_write)
162162
end
163163

164+
def test_wait_mask_writable
165+
omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
166+
assert_equal IO::WRITABLE, @w.wait(IO::WRITABLE, 0)
167+
end
168+
169+
def test_wait_mask_readable
170+
omit("Missing IO::READABLE!") unless IO.const_defined?(:READABLE)
171+
@w.write("Hello World\n" * 3)
172+
assert_equal IO::READABLE, @r.wait(IO::READABLE, 0)
173+
174+
@r.gets
175+
assert_equal IO::READABLE, @r.wait(IO::READABLE, 0)
176+
end
177+
178+
def test_wait_mask_zero
179+
omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
180+
assert_raises(ArgumentError) do
181+
@w.wait(0, 0)
182+
end
183+
end
184+
185+
def test_wait_mask_negative
186+
omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
187+
assert_raises(ArgumentError) do
188+
@w.wait(-6, 0)
189+
end
190+
end
191+
164192
private
165193

166194
def fill_pipe

0 commit comments

Comments
 (0)