Upgrading connection to TLS panicks

[porsager]

I don't know if closed issues are followed, so I'm creating this issue which relates to #9360

I'm experiencing the same as that issue with 1.17.2.

It's happening on my Macbook M1. Here's the complete log:

thread 'main' panicked at 'Only a single use of this resource should happen: FullDuplexResource { rd: AsyncRefCell<tokio::net::tcp::split_owned::OwnedReadHalf>, wr: AsyncRefCell<tokio::net::tcp::split_owned::OwnedWriteHalf>, cancel_handle: CancelHandle { node: Node { inner: UnsafeCell { .. }, _pin: PhantomPinned } } }', /Users/lucacasonato/projects/github.com/denoland/deno/ext/net/ops_tls.rs:721:6
stack backtrace:
   0:        0x105795e18 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h6db16a647ea8ef75
   1:        0x10509ef74 - core::fmt::write::h33ea04624f802921
   2:        0x105795790 - std::io::Write::write_fmt::hd387c43cb670231f
   3:        0x10579522c - std::panicking::default_hook::{{closure}}::hafe36fc92a4bd1da
   4:        0x1057945e0 - std::panicking::rust_panic_with_hook::hcafd722806bfc4bc
   5:        0x1057ae668 - std::panicking::begin_panic_handler::{{closure}}::h857e2d0c4fb1b7ef
   6:        0x1057ae5e4 - std::sys_common::backtrace::__rust_end_short_backtrace::hef54dc67870da455
   7:        0x1057ae5a0 - _rust_begin_unwind
   8:        0x1066d410c - core::panicking::panic_fmt::h70bba9f1fa3fca77
   9:        0x1066d45d4 - core::result::unwrap_failed::h93f88410b7cbb383
  10:        0x10531e1f4 - <futures_util::future::future::Map<Fut,F> as core::future::future::Future>::poll::hd6aa2ce5e4f45400
  11:        0x10524e128 - <futures_util::future::future::Inspect<Fut,F> as core::future::future::Future>::poll::ha1e027248e82e17e
  12:        0x1050dd2c0 - futures_util::stream::stream::StreamExt::poll_next_unpin::hdad5eebc5c23eb7f
  13:        0x1050d97fc - deno_core::runtime::JsRuntime::poll_event_loop::hb092758295959ea7
  14:        0x104f47638 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h1fec428dc61032d8
  15:        0x105050be4 - deno::run_command::{{closure}}::h4380190765d113af
  16:        0x104f61358 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h4fcc6f206a139af5
  17:        0x104f6aea0 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h6be8160c79fb85ff
  18:        0x1050555b8 - deno::main::h68206ac11b06f611
  19:        0x104e13dc4 - std::sys_common::backtrace::__rust_begin_short_backtrace::h6505599a3d7af1ff
  20:        0x105069dd8 - _main

[lucacasonato]

Do you have a reproduction?

[porsager]

Not yet, I'll try to look at it in 5.

[porsager]

Ok so the sample below works in 1.16.4 but not 1.17.x when trying to connect to my local Postgres server. It's not giving me the panic as above though, but rather the error below. I'll try my project that gave the panic with 1.16 now.

deno-ssl $ deno run --unstable --unsafely-ignore-certificate-errors --allow-all index.js                                    5932ms 
DANGER: TLS certificate validation is disabled for all hostnames
Sending fatal alert BadCertificate
error: Uncaught (in promise) InvalidData: invalid peer certificate contents: invalid peer certificate: UnsupportedCertVersion
    at async write (deno:ext/net/01_net.js:26:12)

Code:

Deno.connect({ transport: 'tcp', port: 5432, hostname: 'localhost' }).then(async conn => {
  conn.write(new Uint8Array([0, 0, 0, 8, 4, 210, 22, 47]))
  const b = new Uint8Array(128)

  while (await conn.read(b)) {
    if (b[0] === 83) {
      Deno.startTls(conn, { hostname: 'localhost' }).then(connected)
      break
    }
  }

})

async function connected(conn) {
  const b = new Uint8Array(128)
  while (await conn.read(b))
    console.log(b)
}

[porsager]

Hmm.. also getting the panic on 1.16.4 in my project. I'll look closer tomorrow..

[piscisaureus]

What if you add 'await' before the 'conn.write' call?

[porsager]

No difference

[porsager]

I think the error I'm getting with 1.17.x in the code above is related to #10312 so I won't open a new issue for that.

With regards to the panic I'm still no wiser why I can make a simple repro of it, and why it fails like that in my project.

[porsager]

finally 😅 Here is a reproducible code sample trying to startTls

Deno.connect({ transport: 'tcp', port: 5432, hostname: 'localhost' }).then(async conn => {
  await conn.write(new Uint8Array([0, 0, 0, 8, 4, 210, 22, 47]))
  const b = new Uint8Array(128)

  while (await conn.read(b)) {
    if (b[0] === 83) {
      connect(conn)
      // adding break here fixes the issue
    }
  }

})

async function connect(conn) {
  await 0 // This is the reason it panicks, and if I remove it I get a more descriptive BadResource: Bad resource ID error
  Deno.startTls(conn, { hostname: 'localhost' })
}

It's run with deno run --allow-net index.js

[piscisaureus]

Deno should not crash (that's a bug, it should always fail with BadResource), but I'd like to point out that this causes startTls() to run concurrently with read(). It makes no sense to do that.

[porsager]

Yeah, that also makes sense, but I expected read to end once startTls was called which was why I didn't think I'd need to handle that explicitly.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[porsager]

This still happens with 1.19.3

[kitsonk]

Happening under slightly different conditions in #13926, but the same underlying issue.