jhelovuo / RustDDS

Rust implementation of Data Distribution Service
Apache License 2.0
330 stars 66 forks source link

Avoid WouldBlock error when on Windows #251

Closed ebakoba closed 1 year ago

ebakoba commented 1 year ago

Fixes #249

1) Tried to set .set_nonblocking(false) on mio socket, but this option seemed to not exist. 2) Changed out the mio based socket for the std, which seemed to resolve the problem.

If I had to guess I think mio is setting .set_nonblocking(true) for some reason in its socket implementation.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 12.50% and project coverage change: +0.04 :tada:

Comparison is base (dd4c9bd) 73.35% compared to head (368a832) 73.39%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #251 +/- ## ========================================== + Coverage 73.35% 73.39% +0.04% ========================================== Files 94 94 Lines 18340 18344 +4 ========================================== + Hits 13453 13464 +11 + Misses 4887 4880 -7 ``` | [Impacted Files](https://codecov.io/gh/jhelovuo/RustDDS/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/dds/message\_receiver.rs](https://codecov.io/gh/jhelovuo/RustDDS/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2Rkcy9tZXNzYWdlX3JlY2VpdmVyLnJz) | `73.47% <0.00%> (ø)` | | | [src/messages/submessages/data\_frag.rs](https://codecov.io/gh/jhelovuo/RustDDS/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL21lc3NhZ2VzL3N1Ym1lc3NhZ2VzL2RhdGFfZnJhZy5ycw==) | `0.00% <0.00%> (ø)` | | | [src/serialization/cdr\_deserializer.rs](https://codecov.io/gh/jhelovuo/RustDDS/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3NlcmlhbGl6YXRpb24vY2RyX2Rlc2VyaWFsaXplci5ycw==) | `81.93% <0.00%> (-0.14%)` | :arrow_down: | | [src/network/udp\_sender.rs](https://codecov.io/gh/jhelovuo/RustDDS/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL25ldHdvcmsvdWRwX3NlbmRlci5ycw==) | `82.30% <100.00%> (ø)` | | ... and [5 files with indirect coverage changes](https://codecov.io/gh/jhelovuo/RustDDS/pull/251/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

ebakoba commented 1 year ago

@jhelovuo Seems like the lint is failing due to problems not related to changes in the PR.

ohuopio commented 1 year ago

@ebakoba Hi! Do all the unit tests pass on your Windows machine? On my machine the UDP listener/sender unit tests fail, but on the other hand the Shapes-demo works without problems. So I'm just wondering if the unit tests fail because of something related to my machine.

ebakoba commented 1 year ago

@ohuopio Hi! For me the following tests fail: image

I have tracked it to following function in udp_listener.rs:

  // TODO: remove this. It is used only for tests.
  // We cannot read a single packet only, because we use edge-triggered polls.
  #[cfg(test)]
  pub fn get_message(&self) -> Vec<u8> {
    let mut message: Vec<u8> = vec![];
    let mut buf: [u8; MAX_MESSAGE_SIZE] = [0; MAX_MESSAGE_SIZE];
    match self.socket.recv(&mut buf) {
      Ok(nbytes) => {
        message = buf[..nbytes].to_vec();
        return message;
      }
      Err(e) => {
        debug!("UDPListener::get_message failed: {:?}", e);
      }
    };
    message
  }

This function always fails on Windows with WouldBlock error, same way the sending address used to fail. Unfortunately because of MIO dependency in the listening process we cannot just make it use std::UdpSocket. As the comment mentions this function is only used in tests, thats why the shapes demo works fine. Do you have a suggestion how we could take this further? Could we replace get_message(...) with something more realistic? Do we have to elevate this issue to mio?

ohuopio commented 1 year ago

@ebakoba I'm merging this pull request, since it does fix #249 after all. The 4 unit tests do fail on Windows (as they have failed before), but in the future we'll stop using MIO and start using Rust's standard async features instead. We'll fix the unit tests then.

Thanks for the contribution @ebakoba!