Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transports/{tcp,dns,websocket}: Remove Clone implementation for *Config #2682

Merged
merged 4 commits into from May 31, 2022

Commits on May 30, 2022

  1. transports/{tcp,dns,websocket}: Remove Clone implementation for *Config

    This commit removes the `Clone` implementation on `GenTcpConfig` and consequently the `Clone`
    implementations on `GenDnsConfig` and `WsConfig`.
    
    When port-reuse is enabled, `GenTcpConfig` tracks the addresses it is listening in a `HashSet`. This
    `HashSet` is shared with the `TcpListenStream`s via an `Arc<Mutex<_>>`. Given that `Clone` is
    `derive`d on `GenTcpConfig`, cloning a `GenTcpConfig`, results in both instances sharing the same
    set of listen addresses. This is not intuitive.
    
    This behavior is for example error prone in the scenario where one wants to speak both plain DNS/TCP and
    Websockets. Say a user creates the transport in the following way:
    
    ``` Rust
    let transport = {
        let tcp = tcp::TcpConfig::new().nodelay(true).port_reuse(true);
        let dns_tcp = dns::DnsConfig::system(tcp).await?;
        let ws_dns_tcp = websocket::WsConfig::new(dns_tcp.clone());
        dns_tcp.or_transport(ws_dns_tcp)
    };
    ```
    
    Both `dns_tcp` and `ws_dns_tcp` share the set of listen addresses, given the `dns_tcp.clone()` to
    create the `ws_dns_tcp`. Thus, with port-reuse, a Websocket dial might reuse a DNS/TCP listening
    port instead of a Websocket listening port.
    
    With this commit a user is forced to do the below, preventing the above error:
    
    ``` Rust
    let transport = {
        let dns_tcp = dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true).port_reuse(true)).await?;
        let ws_dns_tcp = websocket::WsConfig::new(
            dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true).port_reuse(true)).await?,
        );
        dns_tcp.or_transport(ws_dns_tcp)
    };
    ```
    mxinden committed May 30, 2022
    Copy the full SHA
    5559a76 View commit details
    Browse the repository at this point in the history
  2. transports/: Reference pull request in changelogs

    Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
    mxinden and thomaseizinger committed May 30, 2022
    Copy the full SHA
    a7d69b6 View commit details
    Browse the repository at this point in the history
  3. transports/: Fix tests

    mxinden committed May 30, 2022
    Copy the full SHA
    2ede3d0 View commit details
    Browse the repository at this point in the history
  4. Copy the full SHA
    3b2c7a0 View commit details
    Browse the repository at this point in the history