Segfault can occur when connection is closed

[wch]

From rstudio/shiny#1967. Sometimes a segfault can happen when an app is closed during loading.

Trace from lldb:

* thread #2, stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
    frame #0: 0x00000001092d796c httpuv.so`WebSocketConnection::markClosed(this=0x0000000000000000) at websockets.cpp:317
   314  
   315  void WebSocketConnection::markClosed() {
   316    ASSERT_BACKGROUND_THREAD()
-> 317    _connState = WS_CLOSED;
   318  }
   319  
   320  void WebSocketConnection::onHeaderComplete(const WSFrameHeaderInfo& header) {
(lldb) bt
* thread #2, stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x00000001092d796c httpuv.so`WebSocketConnection::markClosed(this=0x0000000000000000) at websockets.cpp:317
    frame #1: 0x000000010928c8ac httpuv.so`HttpRequest::_on_closed(this=0x0000000105e0f380, handle=0x0000000000000000) at httprequest.cpp:590
    frame #2: 0x0000000109285b22 httpuv.so`HttpRequest::close(this=0x0000000105e0f380) at httprequest.cpp:604
    frame #3: 0x0000000109291c2a httpuv.so`void boost::_mfi::mf0<void, HttpRequest>::call<boost::shared_ptr<HttpRequest> >(this=0x00000001009e85c0, u=0x00000001009e85d0, (null)=0x0000000000000000) const at mem_fn_template.hpp:40
    frame #4: 0x0000000109291b7c httpuv.so`void boost::_mfi::mf0<void, HttpRequest>::operator(this=0x00000001009e85c0, u=0x00000001009e85d0)<boost::shared_ptr<HttpRequest> >(boost::shared_ptr<HttpRequest>&) const at mem_fn_template.hpp:55
    frame #5: 0x0000000109291b20 httpuv.so`void boost::_bi::list1<boost::_bi::value<boost::shared_ptr<HttpRequest> > >::operator(this=0x00000001009e85d0, (null)=type<void> @ 0x0000700000945648, f=0x00000001009e85c0, a=0x0000700000945670, (null)=0)<boost::_mfi::mf0<void, HttpRequest>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf0<void, HttpRequest>&, boost::_bi::list0&, int) at bind.hpp:259
    frame #6: 0x0000000109291aca httpuv.so`boost::_bi::bind_t<void, boost::_mfi::mf0<void, HttpRequest>, boost::_bi::list1<boost::_bi::value<boost::shared_ptr<HttpRequest> > > >::operator(this=0x00000001009e85c0)() at bind.hpp:1294
    frame #7: 0x0000000109291800 httpuv.so`boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf0<void, HttpRequest>, boost::_bi::list1<boost::_bi::value<boost::shared_ptr<HttpRequest> > > >, void>::invoke(function_obj_ptr=0x0000700000945750) at function_template.hpp:159
    frame #8: 0x0000000109275368 httpuv.so`boost::function0<void>::operator(this=0x0000700000945748)() const at function_template.hpp:759
    frame #9: 0x00000001092765d9 httpuv.so`CallbackQueue::flush(this=0x00000001009b8a70) at callbackqueue.cpp:47
    frame #10: 0x00000001092764b0 httpuv.so`flush_callback_queue(handle=0x00000001009b8a70) at callbackqueue.cpp:12
    frame #11: 0x00000001092e1292 httpuv.so`uv__async_io(loop=0x0000000109322858, w=0x0000000109322ac8, events=1) at async.c:118
    frame #12: 0x00000001092fed9c httpuv.so`uv__io_poll(loop=0x0000000109322858, timeout=-1) at kqueue.c:343
    frame #13: 0x00000001092e196f httpuv.so`uv_run(loop=0x0000000109322858, mode=UV_RUN_DEFAULT) at core.c:368
    frame #14: 0x00000001092a34cf httpuv.so`io_thread(data=0x00007ffeefbf4100) at httpuv.cpp:112
    frame #15: 0x00007fff5c350661 libsystem_pthread.dylib`_pthread_body + 340
    frame #16: 0x00007fff5c35050d libsystem_pthread.dylib`_pthread_start + 377
    frame #17: 0x00007fff5c34fbf9 libsystem_pthread.dylib`thread_start + 13
(lldb) f 1
frame #1: 0x000000010928c8ac httpuv.so`HttpRequest::_on_closed(this=0x0000000105e0f380, handle=0x0000000000000000) at httprequest.cpp:590
   587    // resetting the shared_ptr. This is useful because there may be some
   588    // callbacks that will execute later, and we want to make sure the WSC
   589    // doesn't try to do anything with them.
-> 590    _pWebSocketConnection->markClosed();
   591    _pWebSocketConnection.reset();
   592  }
   593  
(lldb) f 2
frame #2: 0x0000000109285b22 httpuv.so`HttpRequest::close(this=0x0000000105e0f380) at httprequest.cpp:604
   601      // Shouldn't get here, but just in case close() gets called twice
   602      // (probably via a scheduled callback), don't do the closing machinery
   603      // twice.
-> 604      _on_closed(NULL);
   605      return;
   606    }
   607    _is_closing = true;
(lldb) p _is_closing
(bool) $10 = true

I believe the problem happens when HttpRequest::close() is called twice. The logic here does not match the comment, and looks like it's incorrect: https://github.com/rstudio/httpuv/blob/8a28cdb/src/httprequest.cpp#L600-L604

[wch]

I'm able to reproduce this reliably, although it requires some pretty quick timing.

First, run:

R -e "radiant:::radiant_window()"

Next, close the browser window after it is mostly loaded. Then, in the terminal, within ~0.5s, press Ctrl-C. It's easier to do when the Makevars file is edited to enable tracing and thread assertions. I'm then able to get this output reliably:

HttpRequest::_on_headers_complete
HttpRequest::sendWSFrame
on_ws_message_sent
HttpRequest::sendWSFrame
on_ws_message_sent
HttpRequest::close
HttpRequest::_on_closed
WebSocketConnection::~WebSocketConnection
WebSocketConnection::onFrameComplete
HttpRequest::closeWSSocket
HttpRequest::close
HttpRequest::onWSClose
HttpRequest::_on_closed
^CHttpRequest::schedule_close
HttpRequest::close
close() called twice on HttpRequest object
HttpRequest::_on_closed
Assertion failed: (px != 0), function operator->, file /Users/winston/R/3.4/BH/include/boost/smart_ptr/shared_ptr.hpp, line 734.
Abort trap: 6

HttpRequest::close() is indeed getting called twice: Once from HttpRequest::closeWSSocket(), and once from a callback scheduled by HttpRequest::schedule_close().

[wch]

Here's a minimal example that can cause the crash. To do it, run R in two terminals. In the first terminal, start the server:

library(httpuv)
app <- startServer("0.0.0.0", 8000, list(
  onWSOpen = function(ws) {
    ws$onMessage(function(binary, message) {
      message("onMessage start")
      Sys.sleep(5)
      message("onMessage next")
      ws$send(message)
    })
  }
))

In the second terminal, run the following websocket client code (this first requires devtools::install_github('rstudio/websocket')). It will send a message, then close the connection from the client side.

library(websocket)
ws <- WebsocketClient$new("ws://localhost:8000/", function(msg) cat(msg, "\n"))
ws$send("foo")
ws$close()

Finally, within 5 seconds, go to the terminal running the server and press Ctrl-C. This will immediately trigger a segfault.