slowtec / tokio-modbus

A tokio-based modbus library
Apache License 2.0
405 stars 120 forks source link

Panic on unreachable code on disconnect #260

Closed seritools closed 3 months ago

seritools commented 3 months ago

Hi there, just updated from 0.11 to 0.13 in my toy project.

Now receiving this panic on Context::disconnect() at cleanup time:

Disconnecting from peripheral A6:C0:80:92:3F:F9...
thread 'tokio-runtime-worker' panicked at C:\Users\seri\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-modbus-0.13.0\src\frame\mod.rs:244:27:
internal error: entered unreachable code
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/f1586001ace26df7cafeb6534eaf76fb2c5513e5/library\std\src\panicking.rs:658
   1: core::panicking::panic_fmt
             at /rustc/f1586001ace26df7cafeb6534eaf76fb2c5513e5/library\core\src\panicking.rs:74
   2: core::panicking::panic
             at /rustc/f1586001ace26df7cafeb6534eaf76fb2c5513e5/library\core\src\panicking.rs:148
   3: enum2$<tokio_modbus::frame::Request>::function_code
             at C:\Users\seri\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-modbus-0.13.0\src\frame\mod.rs:244
   4: tokio_modbus::service::rtu::impl$0::call::async_fn$0<tokio_serial::SerialStream>
             at C:\Users\seri\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-modbus-0.13.0\src\service\rtu.rs:52
   5: tokio_modbus::service::rtu::impl$3::call::async_block$0<tokio_serial::SerialStream>
             at C:\Users\seri\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-modbus-0.13.0\src\service\rtu.rs:106
   6: core::future::future::impl$1::poll<alloc::boxed::Box<dyn$<core::future::future::Future<assoc$<Output,enum2$<core::result::Result<enum2$<core::result::Result<enum2$<tokio_modbus::frame::Response>,tokio_modbus::frame::Exception> >,enum2$<tokio_modbus::error:
             at /rustc/f1586001ace26df7cafeb6534eaf76fb2c5513e5\library\core\src\future\future.rs:123
   7: tokio_modbus::client::impl$0::disconnect::async_fn$0
             at C:\Users\seri\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-modbus-0.13.0\src\client\mod.rs:90
   ...

I'm using rtu and am on Windows. I can reproduce it with this minimal code:

    let serial_stream = tokio_serial::new(serial_path, 115200)
        .data_bits(DataBits::Eight)
        .stop_bits(StopBits::One)
        .timeout(SERIAL_TIMEOUT)
        .open_native_async()?;

    let mut psu = tokio_modbus::client::rtu::attach_slave(serial_stream, Slave(slave_id));
    psu.disconnect().await??;

Please let me know if you need more information! Thanks!

flosse commented 3 months ago

Here is an explanation of the Disconnect variant https://github.com/slowtec/tokio-modbus/blob/13fcb838b09b0b11d8a26b7099c330ef7e1cb480/src/frame/mod.rs#L178-L187

that causes the panic here:

https://github.com/slowtec/tokio-modbus/blob/13fcb838b09b0b11d8a26b7099c330ef7e1cb480/src/frame/mod.rs#L244

So the question is why is https://github.com/slowtec/tokio-modbus/blob/13fcb838b09b0b11d8a26b7099c330ef7e1cb480/src/frame/mod.rs#L222

called when it must not 🤔

flosse commented 3 months ago

Here we are: https://github.com/slowtec/tokio-modbus/blob/13fcb838b09b0b11d8a26b7099c330ef7e1cb480/src/service/rtu.rs#L51-L52

@uklotzde I think that disconnect needs to be checked before calling function_code() right?

flosse commented 3 months ago

@seritools Thanks for reporting! @uklotzde Thanks for fixing the problem so quickly!

The issue should be resolved now in v0.13.1