Fixed memory leak in do_write function

[s7and]

Memory leak occurs when connection lost and do_write function is called.

==2272== 66 bytes in 2 blocks are definitely lost in loss record 222 of 412
==2272==    at 0x4C298A0: operator new[](unsigned long) (vg_replace_malloc.c:389)
==2272==    by 0x44465B88: wampcc::ssl_socket::write_encrypted_bytes(char const*, unsigned long) (ssl_socket.cc:244)
==2272==    by 0x4446540E: wampcc::ssl_socket::do_encrypt_and_write(char*, unsigned long) (ssl_socket.cc:169)
==2272==    by 0x44464EA8: wampcc::ssl_socket::service_pending_write() (ssl_socket.cc:104)
==2272==    by 0x4446E37A: wampcc::tcp_socket::write(std::pair<char const*, unsigned long>*, unsigned long)::{lambda()#2}::operator()() const (tcp_socket.cc:500)
==2272==    by 0x44473366: std::_Function_handler<void (), wampcc::tcp_socket::write(std::pair<char const*, unsigned long>*, unsigned long)::{lambda()#2}>::_M_invoke(std::_Any_data const&) (std_function.h:316)
==2272==    by 0x44418ADD: std::function<void ()>::operator()() const (std_function.h:706)
==2272==    by 0x44445C88: wampcc::io_loop::on_async() (io_loop.cc:136)
==2272==    by 0x4444544A: wampcc::io_loop::io_loop(wampcc::kernel&, std::function<void ()>)::{lambda(uv_async_s*)#1}::operator()(uv_async_s*) const (io_loop.cc:64)
==2272==    by 0x4444546A: wampcc::io_loop::io_loop(wampcc::kernel&, std::function<void ()>)::{lambda(uv_async_s*)#1}::_FUN(uv_async_s*) (io_loop.cc:65)
==2272==    by 0x3C49AF1B: uv__async_io (async.c:118)
==2272==    by 0x3C4B0411: uv__io_poll (linux-core.c:375)

[darrenjs]

Thanks for this fix & analysis.

However I think there is an additional memory leak in the do_write(std::vector<uv_buf_t> & bufs) function, i.e. if the following pathway is taken:

if (bytes_to_send > (pend_max - m_bytes_pending_write)) {
  LOG_WARN("pending bytes limit reached; closing connection");
  close_once_on_io();
  return;
}

So I think this function can benefit from a scoped memory guard, something similar to what exists in tcp_socket::do_write():

scope_guard buf_guard([&copy]() {
  for (auto& i : copy)
    delete[] i.base;
});

... i.e. set a default memory cleaner that is deactivated upon successful transfer of the memory.

[darrenjs]

Hi, am looking at this additional fix. I think this call to dismiss should take place unconditionally. I.e., once memory has been transferred from the bufs vector to the write_req object, the deletion occurs either in other thread, or, at the point of the delete wr;. So will be a problem if the memory if freed both by the delete wr; and by the buf_guard destructor. Hence the buf_guard must always be deactivated (via call to dismiss).

if (r == 0)
  buf_guard.dismiss(); /* bufs will be deleted in uv callback */

[s7and]

Made it in both do_write functions.

[darrenjs]

And good spot, there was problem in the other do_write function!

[darrenjs]

thanks for fix!