Skip to content

Commit

Permalink
native: Remove a bogus assert in net::read
Browse files Browse the repository at this point in the history
This assert was likely inherited from some point, but it's not quite valid as a
no-timeout read may enter this loop, but data could be stolen by any other read
after the socket is deemed readable.

I saw this fail in a recent bors run where the assertion was tripped.
  • Loading branch information
alexcrichton committed Sep 2, 2014
1 parent 5dfb7a6 commit 2ec7bb8
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/libnative/io/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ pub fn read<T>(fd: sock_t,
// wait for the socket to become readable again.
let _guard = lock();
match retry(|| read(deadline.is_some())) {
-1 if util::wouldblock() => { assert!(deadline.is_some()); }
-1 if util::wouldblock() => {}
-1 => return Err(os::last_error()),
n => { ret = n; break }
}
Expand Down

13 comments on commit 2ec7bb8

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at alexcrichton@2ec7bb8

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/remove-net-assert = 2ec7bb8 into auto

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/remove-net-assert = 2ec7bb8 merged ok, testing candidate = 0a16402d

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at alexcrichton@2ec7bb8

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/remove-net-assert = 2ec7bb8 into auto

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/remove-net-assert = 2ec7bb8 merged ok, testing candidate = 59936909

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at alexcrichton@2ec7bb8

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/remove-net-assert = 2ec7bb8 into auto

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/remove-net-assert = 2ec7bb8 merged ok, testing candidate = aaf141d

@bors
Copy link
Contributor

@bors bors commented on 2ec7bb8 Sep 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = aaf141d

Please sign in to comment.