Config cannot be not be reused

[YuanYuYuan]

Describe the bug

Users could easily make this mistake (?) that reuses the Config as below.

use zenoh::prelude::sync::*;
use zenoh::prelude::Config;


fn main() {
    let (pub_config, sub_config) = {
        let mut common_config = Config::default();
        let locator = "tcp/127.0.0.1:38446";
        common_config
            .scouting
            .multicast
            .set_enabled(Some(false))
            .unwrap();

        let mut pub_config = common_config.clone();
        let mut sub_config = common_config;

        sub_config.listen.endpoints = vec![locator.parse().unwrap()];
        pub_config.connect.endpoints = vec![locator.parse().unwrap()];

        (pub_config, sub_config)
    };

    let pub_session = zenoh::open(pub_config).res().unwrap();
    let sub_session = zenoh::open(sub_config).res().unwrap();
}

• Error log

thread 'main' panicked at zenoh/zenoh/src/net/routing/hat/p2p_peer/gossip.rs:166:23:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at zenoh/zenoh/src/net/routing/hat/p2p_peer/gossip.rs:166:23:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x56f6b0a45616 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h410d4c66be4e37f9
   1:     0x56f6b0a6fac0 - core::fmt::write::he40921d4802ce2ac
   2:     0x56f6b0a427ff - std::io::Write::write_fmt::h5de5a4e7037c9b20
   3:     0x56f6b0a453f4 - std::sys_common::backtrace::print::h11c067a88e3bdb22
   4:     0x56f6b0a46ac7 - std::panicking::default_hook::{{closure}}::h8c832ecb03fde8ea
   5:     0x56f6b0a46829 - std::panicking::default_hook::h1633e272b4150cf3
   6:     0x56f6b0a46f58 - std::panicking::rust_panic_with_hook::hb164d19c0c1e71d4
   7:     0x56f6b0a46df9 - std::panicking::begin_panic_handler::{{closure}}::h0369088c533c20e9
   8:     0x56f6b0a45b16 - std::sys_common::backtrace::__rust_end_short_backtrace::hc11d910daf35ac2e
   9:     0x56f6b0a46b84 - rust_begin_unwind
  10:     0x56f6af9b50b5 - core::panicking::panic_fmt::ha6effc2775a0749c
  11:     0x56f6af9b5173 - core::panicking::panic::h44790a89027c670f
  12:     0x56f6af9b5006 - core::option::unwrap_failed::hcb3a256a9f1ca882
  13:     0x56f6afbe9184 - <petgraph::graph_impl::stable_graph::StableGraph<N,E,Ty,Ix> as core::ops::index::Index<petgraph::graph_impl::NodeIndex<Ix>>>::index::h1207a9cc8c9ebf7d
  14:     0x56f6afd64e9f - zenoh::net::routing::hat::p2p_peer::gossip::Network::make_link_state::h7786e5d4cf908f05
  15:     0x56f6afd6552b - zenoh::net::routing::hat::p2p_peer::gossip::Network::make_msg::h3986cdc030ac0d5f
  16:     0x56f6afc96ebc - zenoh::net::routing::hat::p2p_peer::gossip::Network::send_on_links::ha6a4f31c7d067b9a
  17:     0x56f6afd6d49e - zenoh::net::routing::hat::p2p_peer::gossip::Network::remove_link::h25334ccc3e38c92d
  18:     0x56f6afd352d1 - <zenoh::net::routing::hat::p2p_peer::HatCode as zenoh::net::routing::hat::HatBaseTrait>::closing::h48f77eeb28e07065
  19:     0x56f6afbf5505 - <zenoh::net::primitives::demux::DeMux as zenoh_transport::TransportPeerEventHandler>::closing::h7c37e5f1a7e4f8b2
  20:     0x56f6afb8754c - <zenoh::net::runtime::RuntimeSession as zenoh_transport::TransportPeerEventHandler>::closing::h267601355d03359b
  21:     0x56f6aff1c603 - zenoh_transport::unicast::universal::transport::TransportUnicastUniversal::delete::{{closure}}::h5cf5539312ed1791
  22:     0x56f6aff2397a - <zenoh_transport::unicast::universal::transport::TransportUnicastUniversal as zenoh_transport::unicast::transport_unicast_inner::TransportUnicastTrait>::close::{{closure}}::hb1dd2c6b63e7d45f
  23:     0x56f6afd9cfad - <core::pin::Pin<P> as core::future::future::Future>::poll::hda41d91471133211
  24:     0x56f6afabf489 - zenoh_transport::unicast::manager::<impl zenoh_transport::manager::TransportManager>::close_unicast::{{closure}}::ha2726d7bfb0bf79b
  25:     0x56f6afc67909 - zenoh_transport::manager::TransportManager::close::{{closure}}::h231279b59c066b29
  26:     0x56f6afaae6a5 - zenoh::net::runtime::Runtime::close::{{closure}}::hc444ba8b2527db2a
  27:     0x56f6afb7ae24 - zenoh::session::Session::close::{{closure}}::h187bbcb98521596c
  28:     0x56f6afcbc76b - tokio::runtime::park::CachedParkThread::block_on::{{closure}}::h3129172af4bc240f
  29:     0x56f6afcbb75e - tokio::runtime::park::CachedParkThread::block_on::ha6e94d37579aae13
  30:     0x56f6afb1b97e - tokio::runtime::context::blocking::BlockingRegionGuard::block_on::he8b1f6641b456e54
  31:     0x56f6afd216a5 - tokio::runtime::handle::Handle::block_on::{{closure}}::h1ad6d3515ef70fcb
  32:     0x56f6afc2d0c7 - tokio::runtime::context::runtime::enter_runtime::h672407030ac4e05e
  33:     0x56f6afd215e6 - tokio::runtime::handle::Handle::block_on::h769665db19327512
  34:     0x56f6afccac86 - zenoh_runtime::ZRuntime::block_in_place::{{closure}}::hf83470bc1e76e2bb
  35:     0x56f6afce7c3d - tokio::runtime::scheduler::multi_thread::worker::block_in_place::h2419c6bb3435fbc4
  36:     0x56f6afac26da - tokio::runtime::scheduler::block_in_place::block_in_place::h1f2f6e956eb3290b
  37:     0x56f6afbf345a - tokio::task::blocking::block_in_place::hfa1c177da5f9fdab
  38:     0x56f6afcca4ec - zenoh_runtime::ZRuntime::block_in_place::h8c22adf8c70baf45
  39:     0x56f6afa87427 - <zenoh_core::ResolveFuture<F,To> as zenoh_core::SyncResolve>::res_sync::hbd67c90edee81c05
  40:     0x56f6afaad249 - <zenoh::session::Session as core::ops::drop::Drop>::drop::ha2522dd147517422
  41:     0x56f6afa9ab91 - core::ptr::drop_in_place<zenoh::session::Session>::h9d31908e6a444eb7
  42:     0x56f6afa385be - zenoh_quick_testbed::main::heb24d3d640c32dd7
  43:     0x56f6afa0e753 - core::ops::function::FnOnce::call_once::hcc88671ca69f9e01
  44:     0x56f6afa0cdc6 - std::sys_common::backtrace::__rust_begin_short_backtrace::h296dd4ea15850a64
  45:     0x56f6af9b7ea9 - std::rt::lang_start::{{closure}}::h45f084a6ddd51a70
  46:     0x56f6b0a3d501 - std::rt::lang_start_internal::h4d236095b69a230b
  47:     0x56f6af9b7e87 - std::rt::lang_start::h555e8ebb611b2354
  48:     0x56f6afa387a5 - main
  49:     0x77d8fafc1cd0 - <unknown>
  50:     0x77d8fafc1d8a - __libc_start_main
  51:     0x56f6af9b59f5 - _start
  52:                0x0 - <unknown>
thread 'main' panicked at library/core/src/panicking.rs:163:5:
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.

The reason behind this is Config has a unique session id and cannot be reused to construct another session. We might need to consider

1. Either prevent users from doing so.
2. Split the session ID inside the config.
3. Improve the error report rather than a panic error or maybe the gossip behavior.

To reproduce

See above.

System info

• Zenoh commit: ea604b6

[YuanYuYuan]

@Mallets @OlivierHecart do you mind leaving a comment? Thanks.

[milyin]

So the problem is that session can't be created because of session id duplication? Then the right behaviour should for zenoh::open should be returning ZResult::Err with proper error message I think. Of course panic deep inside is wrong, this should be fixed.
So I think variant (3) is right.

[Mallets]

It seems that more than zenoh::open should return a ZResult::Err, it's gossip that should handle the case of duplicated ZenohId.

In any case, I believe the session ID should be create inside zenoh::open and not when calling Config::default().
In fact, Config::default() should set the internal config value to None instead of setting it to a specific value.

[YuanYuYuan]

So the problem is that session can't be created because of session id duplication?

@milyin Nope. If you append a println!("DONE") at the end of the file, you can observe the message before the panic. The error is rooted in the termination process.