Skip to content

Commit 63cd4ac

Browse files
committed
std: Clarify what timers do with zero and negative durations
Add tests. Also fix a bunch of broken time tests.
1 parent 734834c commit 63cd4ac

File tree

6 files changed

+111
-31
lines changed

6 files changed

+111
-31
lines changed

src/libstd/io/process.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,7 @@ mod tests {
976976
assert!(!p.wait().unwrap().success());
977977
return
978978
}
979-
timer::sleep_ms(100);
979+
timer::sleep(Duration::milliseconds(100));
980980
}
981981
fail!("never saw the child go away");
982982
})

src/libstd/io/signal.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ mod test_unix {
167167
use comm::Empty;
168168
use io::timer;
169169
use super::{Listener, Interrupt};
170+
use time::Duration;
170171

171172
fn sigint() {
172173
unsafe {
@@ -179,7 +180,7 @@ mod test_unix {
179180
let mut signal = Listener::new();
180181
signal.register(Interrupt).unwrap();
181182
sigint();
182-
timer::sleep_ms(10);
183+
timer::sleep(Duration::milliseconds(10));
183184
match signal.rx.recv() {
184185
Interrupt => (),
185186
s => fail!("Expected Interrupt, got {:?}", s),
@@ -193,7 +194,7 @@ mod test_unix {
193194
s1.register(Interrupt).unwrap();
194195
s2.register(Interrupt).unwrap();
195196
sigint();
196-
timer::sleep_ms(10);
197+
timer::sleep(Duration::milliseconds(10));
197198
match s1.rx.recv() {
198199
Interrupt => (),
199200
s => fail!("Expected Interrupt, got {:?}", s),
@@ -212,7 +213,7 @@ mod test_unix {
212213
s2.register(Interrupt).unwrap();
213214
s2.unregister(Interrupt);
214215
sigint();
215-
timer::sleep_ms(10);
216+
timer::sleep(Duration::milliseconds(10));
216217
assert_eq!(s2.rx.try_recv(), Err(Empty));
217218
}
218219
}

src/libstd/io/test.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ macro_rules! iotest (
3939
use io::process::*;
4040
use rt::running_on_valgrind;
4141
use str;
42+
use time::Duration;
4243

4344
fn f() $b
4445

src/libstd/io/timer.rs

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ and create receivers which will receive notifications after a period of time.
1717
1818
*/
1919

20+
// FIXME: These functions take Durations but only pass ms to the backend impls.
21+
2022
use comm::{Receiver, Sender, channel};
2123
use time::Duration;
2224
use io::{IoResult, IoError};
@@ -71,13 +73,15 @@ pub struct Timer {
7173
struct TimerCallback { tx: Sender<()> }
7274

7375
fn in_ms(d: Duration) -> u64 {
74-
// FIXME: Do we really want to fail on negative duration?
7576
let ms = d.num_milliseconds();
76-
if ms < 0 { fail!("negative duration") }
77+
if ms < 0 { return 0 };
7778
return ms as u64;
7879
}
7980

8081
/// Sleep the current task for the specified duration.
82+
///
83+
/// When provided a zero or negative `duration`, the function will
84+
/// return immediately.
8185
pub fn sleep(duration: Duration) {
8286
let timer = Timer::new();
8387
let mut timer = timer.ok().expect("timer::sleep: could not create a Timer");
@@ -99,8 +103,14 @@ impl Timer {
99103
///
100104
/// Note that this function will cause any other receivers for this timer to
101105
/// be invalidated (the other end will be closed).
106+
///
107+
/// When provided a zero or negative `duration`, the function will
108+
/// return immediately.
102109
pub fn sleep(&mut self, duration: Duration) {
103-
self.obj.sleep(in_ms(duration));
110+
// Short-circuit the timer backend for 0 duration
111+
let ms = in_ms(duration);
112+
if ms == 0 { return }
113+
self.obj.sleep(ms);
104114
}
105115

106116
/// Creates a oneshot receiver which will have a notification sent when
@@ -137,9 +147,17 @@ impl Timer {
137147
/// // The timer object was destroyed, so this will always fail:
138148
/// // five_ms.recv()
139149
/// ```
150+
///
151+
/// When provided a zero or negative `duration`, the message will
152+
/// be sent immediately.
140153
pub fn oneshot(&mut self, duration: Duration) -> Receiver<()> {
141154
let (tx, rx) = channel();
142-
self.obj.oneshot(in_ms(duration), box TimerCallback { tx: tx });
155+
// Short-circuit the timer backend for 0 duration
156+
if in_ms(duration) != 0 {
157+
self.obj.oneshot(in_ms(duration), box TimerCallback { tx: tx });
158+
} else {
159+
tx.send(());
160+
}
143161
return rx
144162
}
145163

@@ -185,9 +203,17 @@ impl Timer {
185203
/// // The timer object was destroyed, so this will always fail:
186204
/// // five_ms.recv()
187205
/// ```
206+
///
207+
/// When provided a zero or negative `duration`, the messages will
208+
/// be sent without delay.
188209
pub fn periodic(&mut self, duration: Duration) -> Receiver<()> {
210+
let ms = in_ms(duration);
211+
// FIXME: The backend implementations don't ever send a message
212+
// if given a 0 ms duration. Temporarily using 1ms. It's
213+
// not clear what use a 0ms period is anyway...
214+
let ms = if ms == 0 { 1 } else { ms };
189215
let (tx, rx) = channel();
190-
self.obj.period(in_ms(duration), box TimerCallback { tx: tx });
216+
self.obj.period(ms, box TimerCallback { tx: tx });
191217
return rx
192218
}
193219
}
@@ -200,8 +226,6 @@ impl Callback for TimerCallback {
200226

201227
#[cfg(test)]
202228
mod test {
203-
use time::Duration;
204-
205229
iotest!(fn test_io_timer_sleep_simple() {
206230
let mut timer = Timer::new().unwrap();
207231
timer.sleep(Duration::milliseconds(1));
@@ -214,20 +238,20 @@ mod test {
214238

215239
iotest!(fn test_io_timer_sleep_oneshot_forget() {
216240
let mut timer = Timer::new().unwrap();
217-
timer.oneshot(Duration::milliseconds(100000000000));
241+
timer.oneshot(Duration::milliseconds(100000000));
218242
})
219243

220244
iotest!(fn oneshot_twice() {
221245
let mut timer = Timer::new().unwrap();
222246
let rx1 = timer.oneshot(Duration::milliseconds(10000));
223-
let rx = timer.oneshot(1);
247+
let rx = timer.oneshot(Duration::milliseconds(1));
224248
rx.recv();
225249
assert_eq!(rx1.recv_opt(), Err(()));
226250
})
227251

228252
iotest!(fn test_io_timer_oneshot_then_sleep() {
229253
let mut timer = Timer::new().unwrap();
230-
let rx = timer.oneshot(Duration::milliseconds(100000000000));
254+
let rx = timer.oneshot(Duration::milliseconds(100000000));
231255
timer.sleep(Duration::milliseconds(1)); // this should invalidate rx
232256

233257
assert_eq!(rx.recv_opt(), Err(()));
@@ -243,7 +267,7 @@ mod test {
243267

244268
iotest!(fn test_io_timer_sleep_periodic_forget() {
245269
let mut timer = Timer::new().unwrap();
246-
timer.periodic(Duration::milliseconds(100000000000));
270+
timer.periodic(Duration::milliseconds(100000000));
247271
})
248272

249273
iotest!(fn test_io_timer_sleep_standalone() {
@@ -277,7 +301,7 @@ mod test {
277301
let rx = timer.periodic(Duration::milliseconds(1));
278302
rx.recv();
279303
rx.recv();
280-
let rx2 = timer.periodic(Durtion::milliseconds(1));
304+
let rx2 = timer.periodic(Duration::milliseconds(1));
281305
rx2.recv();
282306
rx2.recv();
283307
})
@@ -375,4 +399,45 @@ mod test {
375399
// callback do something terrible.
376400
timer2.sleep(Duration::milliseconds(2));
377401
})
402+
403+
iotest!(fn sleep_zero() {
404+
let mut timer = Timer::new().unwrap();
405+
timer.sleep(Duration::milliseconds(0));
406+
})
407+
408+
iotest!(fn sleep_negative() {
409+
let mut timer = Timer::new().unwrap();
410+
timer.sleep(Duration::milliseconds(-1000000));
411+
})
412+
413+
iotest!(fn oneshot_zero() {
414+
let mut timer = Timer::new().unwrap();
415+
let rx = timer.oneshot(Duration::milliseconds(0));
416+
rx.recv();
417+
})
418+
419+
iotest!(fn oneshot_negative() {
420+
let mut timer = Timer::new().unwrap();
421+
let rx = timer.oneshot(Duration::milliseconds(-1000000));
422+
rx.recv();
423+
})
424+
425+
iotest!(fn periodic_zero() {
426+
let mut timer = Timer::new().unwrap();
427+
let rx = timer.periodic(Duration::milliseconds(0));
428+
rx.recv();
429+
rx.recv();
430+
rx.recv();
431+
rx.recv();
432+
})
433+
434+
iotest!(fn periodic_negative() {
435+
let mut timer = Timer::new().unwrap();
436+
let rx = timer.periodic(Duration::milliseconds(-1000000));
437+
rx.recv();
438+
rx.recv();
439+
rx.recv();
440+
rx.recv();
441+
})
442+
378443
}

src/libstd/task.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,10 +664,11 @@ mod test {
664664
#[test]
665665
fn task_abort_no_kill_runtime() {
666666
use std::io::timer;
667+
use time::Duration;
667668
use mem;
668669

669670
let tb = TaskBuilder::new();
670671
let rx = tb.try_future(proc() {});
671672
mem::drop(rx);
672-
timer::sleep_ms(1000);
673+
timer::sleep(Duration::milliseconds(1000));
673674
}

src/libstd/time.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -497,12 +497,16 @@ fn div_rem_64(this: i64, other: i64) -> (i64, i64) {
497497
#[cfg(test)]
498498
mod tests {
499499
use super::{Duration, MIN_DAYS, MAX_DAYS, MIN_DURATION, MAX_DURATION};
500-
use std::{i32, i64};
500+
use {i32, i64};
501+
use num::{Zero, CheckedAdd, CheckedSub};
502+
use option::{Some, None};
503+
use to_string::ToString;
501504

502505
#[test]
503506
fn test_duration() {
504-
assert_eq!(Duration::zero(), Duration::zero());
505-
assert!(Duration::zero() != Duration::seconds(1));
507+
let d: Duration = Zero::zero();
508+
assert_eq!(d, Zero::zero());
509+
assert!(Duration::seconds(1) != Zero::zero());
506510
assert_eq!(Duration::seconds(1) + Duration::seconds(2), Duration::seconds(3));
507511
assert_eq!(Duration::seconds(86399) + Duration::seconds(4),
508512
Duration::days(1) + Duration::seconds(3));
@@ -517,7 +521,8 @@ mod tests {
517521

518522
#[test]
519523
fn test_duration_num_days() {
520-
assert_eq!(Duration::zero().num_days(), 0);
524+
let d: Duration = Zero::zero();
525+
assert_eq!(d.num_days(), 0);
521526
assert_eq!(Duration::days(1).num_days(), 1);
522527
assert_eq!(Duration::days(-1).num_days(), -1);
523528
assert_eq!(Duration::seconds(86399).num_days(), 0);
@@ -534,7 +539,8 @@ mod tests {
534539

535540
#[test]
536541
fn test_duration_num_seconds() {
537-
assert_eq!(Duration::zero().num_seconds(), 0);
542+
let d: Duration = Zero::zero();
543+
assert_eq!(d.num_seconds(), 0);
538544
assert_eq!(Duration::seconds(1).num_seconds(), 1);
539545
assert_eq!(Duration::seconds(-1).num_seconds(), -1);
540546
assert_eq!(Duration::milliseconds(999).num_seconds(), 0);
@@ -551,7 +557,8 @@ mod tests {
551557

552558
#[test]
553559
fn test_duration_num_milliseconds() {
554-
assert_eq!(Duration::zero().num_milliseconds(), 0);
560+
let d: Duration = Zero::zero();
561+
assert_eq!(d.num_milliseconds(), 0);
555562
assert_eq!(Duration::milliseconds(1).num_milliseconds(), 1);
556563
assert_eq!(Duration::milliseconds(-1).num_milliseconds(), -1);
557564
assert_eq!(Duration::microseconds(999).num_milliseconds(), 0);
@@ -568,7 +575,8 @@ mod tests {
568575

569576
#[test]
570577
fn test_duration_num_microseconds() {
571-
assert_eq!(Duration::zero().num_microseconds(), Some(0));
578+
let d: Duration = Zero::zero();
579+
assert_eq!(d.num_microseconds(), Some(0));
572580
assert_eq!(Duration::microseconds(1).num_microseconds(), Some(1));
573581
assert_eq!(Duration::microseconds(-1).num_microseconds(), Some(-1));
574582
assert_eq!(Duration::nanoseconds(999).num_microseconds(), Some(0));
@@ -594,7 +602,8 @@ mod tests {
594602

595603
#[test]
596604
fn test_duration_num_nanoseconds() {
597-
assert_eq!(Duration::zero().num_nanoseconds(), Some(0));
605+
let d: Duration = Zero::zero();
606+
assert_eq!(d.num_nanoseconds(), Some(0));
598607
assert_eq!(Duration::nanoseconds(1).num_nanoseconds(), Some(1));
599608
assert_eq!(Duration::nanoseconds(-1).num_nanoseconds(), Some(-1));
600609
assert_eq!(Duration::new(1, 2, 3_004_005).num_nanoseconds(), Some(86402_003_004_005));
@@ -627,9 +636,10 @@ mod tests {
627636

628637
#[test]
629638
fn test_duration_mul() {
630-
assert_eq!(Duration::zero() * i32::MAX, Duration::zero());
631-
assert_eq!(Duration::zero() * i32::MIN, Duration::zero());
632-
assert_eq!(Duration::nanoseconds(1) * 0, Duration::zero());
639+
let d: Duration = Zero::zero();
640+
assert_eq!(d * i32::MAX, d);
641+
assert_eq!(d * i32::MIN, d);
642+
assert_eq!(Duration::nanoseconds(1) * 0, Zero::zero());
633643
assert_eq!(Duration::nanoseconds(1) * 1, Duration::nanoseconds(1));
634644
assert_eq!(Duration::nanoseconds(1) * 1_000_000_000, Duration::seconds(1));
635645
assert_eq!(Duration::nanoseconds(1) * -1_000_000_000, -Duration::seconds(1));
@@ -642,8 +652,9 @@ mod tests {
642652

643653
#[test]
644654
fn test_duration_div() {
645-
assert_eq!(Duration::zero() / i32::MAX, Duration::zero());
646-
assert_eq!(Duration::zero() / i32::MIN, Duration::zero());
655+
let d: Duration = Zero::zero();
656+
assert_eq!(d / i32::MAX, d);
657+
assert_eq!(d / i32::MIN, d);
647658
assert_eq!(Duration::nanoseconds(123_456_789) / 1, Duration::nanoseconds(123_456_789));
648659
assert_eq!(Duration::nanoseconds(123_456_789) / -1, -Duration::nanoseconds(123_456_789));
649660
assert_eq!(-Duration::nanoseconds(123_456_789) / -1, Duration::nanoseconds(123_456_789));
@@ -652,7 +663,8 @@ mod tests {
652663

653664
#[test]
654665
fn test_duration_fmt() {
655-
assert_eq!(Duration::zero().to_string(), "PT0S".to_string());
666+
let d: Duration = Zero::zero();
667+
assert_eq!(d.to_string(), "PT0S".to_string());
656668
assert_eq!(Duration::days(42).to_string(), "P42D".to_string());
657669
assert_eq!(Duration::days(-42).to_string(), "P-42D".to_string());
658670
assert_eq!(Duration::seconds(42).to_string(), "PT42S".to_string());

0 commit comments

Comments
 (0)