fix use after move in websocket handshake method

[TheCoconutChef]

This fixes an issue in which the socket would never return the response string during the handshake phase of a websocket connection initialization.

This was due to the fact that the response_str variable was moved into the capture scope of the WriteHandler lambda passed as the third argument to async_write.

Since this variable isn' used within the context of the lambda, there doesn't seem to be need for it anyhow.

[stephenberry]

@TheCoconutChef, thanks so much for reporting this bug. I just pushed a tweak that fixes potential use of deleted memory in your code. Because the async_write is non-blocking we need to make sure that when we exit scope the memory isn't destroyed. We use this std::shared_ptr approach in the http_server, but I had missed using it for the websocket code. This captures the shared_ptr to the data in the lambda so that the memory is alive as long as the async_write call is active.

[TheCoconutChef]

I see that you're right.

But I do wonder whether it wouldn't be preferable to directly create a shared_ptr of either std::string or std::vector<uint8_t> as opposed to first creating, then moving them. I did some benchmark to confirm this using nanobench and I have the following result:

| relative |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
|---------:|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|   100.0% |              913.50 |        1,094,693.95 |    1.4% |       10,353.00 |        2,274.76 |  4.551 |       2,078.00 |    0.0% |      1.32 | `create_shared_directly`
|   111.5% |              819.15 |        1,220,775.31 |    1.2% |        6,373.00 |        2,034.01 |  3.133 |       2,080.00 |    0.0% |      1.20 | `create_and_move_to_shared`

I'm using the following code:

std::shared_ptr<std::vector<uint8_t>> create_and_move(int n)
{
  auto v = std::vector<uint8_t>(n);
  for (auto i = 0u; i < v.size(); ++i)
  {
    v[i] = i;
  }
  return std::make_shared<std::vector<uint8_t>>(std::move(v));
}

std::shared_ptr<std::vector<uint8_t>> create_directly(int n)
{
  auto v = std::make_shared<std::vector<uint8_t>>(n);
  auto& vv = *v;
  for (auto i = 0u; i < v->size(); ++i)
  {
    vv[i] = i;
  }
  return v;
}

int main()
{
  auto bench = ankerl::nanobench::Bench();
  bench.title.warmup(100).relative(true).minEpochIterations(132040);

  bench.run("create_shared_directly",
            [&]
            {
              ankerl::nanobench::doNotOptimizeAway(create_directly(1000));
            });

  bench.run("create_and_move_to_shared",
            [&]
            {
              ankerl::nanobench::doNotOptimizeAway(create_and_move(1000));
            });

  return 0;
}                                                                         

Edit: modified code so the diff is less drastic, but still present.

[stephenberry]

Thanks for the benchmarking, I'd be happy to merge a pull request where the shared_ptr is just created directly. In the long run we might want to use buffer pools to avoid these allocations, but this is a simple improvement that I think is good.