Revert "replace native implementation of curl_multi_await with a syst…

…emlib one" (#8313)

Summary:
fixes #8156
refs #7485
refs #8043

The previous stability issues with the native impl were likely not
actually an issue with async curl, but with async more generally with
libevent2.

93c31ec appears to have fixed the original stability issues (and the issues with
stream_await()); it just looked like `curl_multi_await()` and
`stream_await()` were buggy as:
- FB rarely uses these functions (in particular, for our use case, async
curl is generaly less performant than sync curl - but that depends on
what other load you have, and how long requests take)
- FB uses libevent1
- `curl_multi_await()` and `stream_await()` are among the most useful
external features

... so, an HHVM issue when using libevent2 looks like a curl + stream
async issue

Pull Request resolved: #8313

Reviewed By: jano

Differential Revision: D9648538

Pulled By: fredemmott

fbshipit-source-id: 4c1b7bb9a77fbcdff9c28211c2f22f1b4ead5930

hphp/runtime/ext/curl/config.cmake

@@ -1,11 +1,13 @@
 HHVM_DEFINE_EXTENSION("curl"
   SOURCES
+    curl-multi-await.cpp
     curl-multi-resource.cpp
     curl-pool.cpp
     curl-resource.cpp
     ext_curl.cpp
     curl-share-resource.cpp
   HEADERS
+    curl-multi-await.h
     curl-multi-resource.h
     curl-pool.h
     curl-resource.h

hphp/runtime/ext/curl/curl-multi-await.cpp

@@ -0,0 +1,152 @@
+#include "hphp/runtime/ext/curl/curl-multi-await.h"
+#include "hphp/runtime/ext/curl/curl-resource.h"
+#include "hphp/runtime/ext/asio/socket-event.h"
+
+#include "hphp/util/compatibility.h"
+
+namespace HPHP {
+/////////////////////////////////////////////////////////////////////////////
+// CurlEventHandler
+
+struct CurlEventHandler : AsioEventHandler {
+  CurlEventHandler(AsioEventBase* base, int fd, CurlMultiAwait* cma):
+    AsioEventHandler(base, fd), m_curlMultiAwait(cma), m_fd(fd) {}
+
+  void handlerReady(uint16_t /*events*/) noexcept override {
+    m_curlMultiAwait->setFinished(m_fd);
+  }
+
+  TYPE_SCAN_IGNORE_BASES(folly::EventHandler);
+
+ private:
+  CurlMultiAwait* m_curlMultiAwait;
+  int m_fd;
+};
+
+/////////////////////////////////////////////////////////////////////////////
+// CurlTimeoutHandler
+
+struct CurlTimeoutHandler : AsioTimeoutHandler {
+  CurlTimeoutHandler(AsioEventBase* base, CurlMultiAwait* cma):
+    AsioTimeoutHandler(base), m_curlMultiAwait(cma) {}
+
+  void timeoutExpired() noexcept override {
+    m_curlMultiAwait->setFinished(-1);
+  }
+
+  TYPE_SCAN_IGNORE_BASES(folly::AsyncTimeout);
+ private:
+  CurlMultiAwait* m_curlMultiAwait;
+};
+
+/////////////////////////////////////////////////////////////////////////////
+// CurlMultiAwait
+
+CurlMultiAwait::CurlMultiAwait(req::ptr<CurlMultiResource> multi,
+                               double timeout) {
+ if ((addLowHandles(multi) + addHighHandles(multi)) == 0) {
+    // Nothing to do
+    markAsFinished();
+    return;
+  }
+
+  // Add optional timeout
+  int64_t timeout_ms = timeout * 1000;
+  if (timeout_ms > 0) {
+    auto asio_event_base = getSingleton<AsioEventBase>();
+    m_timeout = req::make_shared<CurlTimeoutHandler>(asio_event_base.get(),
+                                                     this);
+
+    asio_event_base->runInEventBaseThreadAndWait([this,timeout_ms] {
+      m_timeout->scheduleTimeout(timeout_ms);
+    });
+  }
+}
+
+CurlMultiAwait::~CurlMultiAwait() {
+  for (auto handler : m_handlers) {
+    handler->unregisterHandler();
+  }
+  if (m_timeout) {
+    auto asio_event_base = getSingleton<AsioEventBase>();
+    auto to = std::move(m_timeout);
+    asio_event_base->runInEventBaseThreadAndWait([to] {
+      to.get()->cancelTimeout();
+    });
+  }
+  m_handlers.clear();
+}
+
+void CurlMultiAwait::unserialize(Cell& c) {
+  c.m_type = KindOfInt64;
+  c.m_data.num = m_result;
+}
+
+void CurlMultiAwait::setFinished(int fd) {
+  if (m_result < fd) {
+    m_result = fd;
+  }
+  if (!m_finished) {
+    markAsFinished();
+    m_finished = true;
+  }
+}
+
+void CurlMultiAwait::addHandle(int fd, int events) {
+  auto asio_event_base = getSingleton<AsioEventBase>();
+  auto handler =
+    req::make_shared<CurlEventHandler>(asio_event_base.get(), fd, this);
+  handler->registerHandler(events);
+  m_handlers.push_back(handler);
+}
+
+// Ask curl_multi for its handles directly
+// This is preferable as we get to know which
+// are blocking on reads, and which on writes.
+int CurlMultiAwait::addLowHandles(req::ptr<CurlMultiResource> multi) {
+  fd_set read_fds, write_fds;
+  int max_fd = -1, count = 0;
+  FD_ZERO(&read_fds); FD_ZERO(&write_fds);
+  if ((CURLM_OK != curl_multi_fdset(multi->get(), &read_fds, &write_fds,
+                                    nullptr, &max_fd)) ||
+      (max_fd < 0)) {
+    return count;
+  }
+  for (int i = 0 ; i <= max_fd; ++i) {
+    int events = 0;
+    if (FD_ISSET(i, &read_fds))  events |= AsioEventHandler::READ;
+    if (FD_ISSET(i, &write_fds)) events |= AsioEventHandler::WRITE;
+    if (events) {
+      addHandle(i, events);
+      ++count;
+    }
+  }
+  return count;
+}
+
+// Check for file descriptors >= FD_SETSIZE
+// which can't be returned in an fdset
+// This is a little hacky, but necessary given cURL's APIs
+int CurlMultiAwait::addHighHandles(req::ptr<CurlMultiResource> multi) {
+  int count = 0;
+  auto easy_handles = multi->getEasyHandles();
+  for (ArrayIter iter(easy_handles); iter; ++iter) {
+    Variant easy_handle = iter.second();
+    auto easy = dyn_cast_or_null<CurlResource>(easy_handle);
+    if (!easy) continue;
+    long sock;
+    if ((curl_easy_getinfo(easy->get(),
+                           CURLINFO_LASTSOCKET, &sock) != CURLE_OK) ||
+        (sock < FD_SETSIZE)) {
+      continue;
+    }
+    // No idea which type of event it needs, ask for everything
+    addHandle(sock, AsioEventHandler::READ_WRITE);
+    ++count;
+  }
+  return count;
+}
+
+
+/////////////////////////////////////////////////////////////////////////////
+}

hphp/runtime/ext/curl/curl-multi-await.h

@@ -0,0 +1,36 @@
+#ifndef incl_HPHP_CURL_MULTI_AWAIT_H
+#define incl_HPHP_CURL_MULTI_AWAIT_H
+
+#include "hphp/runtime/ext/asio/asio-external-thread-event.h"
+#include "hphp/runtime/ext/curl/curl-multi-resource.h"
+
+namespace HPHP {
+/////////////////////////////////////////////////////////////////////////////
+
+struct CurlEventHandler;
+struct CurlTimeoutHandler;
+
+struct CurlMultiAwait : AsioExternalThreadEvent {
+  CurlMultiAwait(req::ptr<CurlMultiResource> multi, double timeout);
+  ~CurlMultiAwait();
+
+  void unserialize(Cell& c) override;
+
+ private:
+  friend struct CurlEventHandler;
+  friend struct CurlTimeoutHandler;
+  void setFinished(int fd);
+
+  void addHandle(int fd, int events);
+  int addLowHandles(req::ptr<CurlMultiResource> multi);
+  int addHighHandles(req::ptr<CurlMultiResource> multi);
+
+  req::shared_ptr<CurlTimeoutHandler> m_timeout;
+  req::vector<req::shared_ptr<CurlEventHandler>> m_handlers;
+  int m_result{-1};
+  bool m_finished{false};
+};
+
+/////////////////////////////////////////////////////////////////////////////
+}
+#endif

hphp/runtime/ext/curl/ext_curl.cpp

@@ -16,6 +16,7 @@
 */
 
 #include "hphp/runtime/ext/curl/ext_curl.h"
+#include "hphp/runtime/ext/curl/curl-multi-await.h"
 #include "hphp/runtime/ext/curl/curl-multi-resource.h"
 #include "hphp/runtime/ext/curl/curl-pool.h"
 #include "hphp/runtime/ext/curl/curl-resource.h"
@@ -610,6 +611,19 @@ Variant HHVM_FUNCTION(curl_multi_select, const Resource& mh,
   return ret;
 }
 
+Object HHVM_FUNCTION(curl_multi_await, const Resource& mh,
+                                       double timeout /*=1.0*/) {
+  CHECK_MULTI_RESOURCE_THROW(curlm);
+  auto ev = new CurlMultiAwait(curlm, timeout);
+  try {
+    return Object{ev->getWaitHandle()};
+  } catch (...) {
+    assert(false);
+    ev->abandon();
+    throw;
+  }
+}
+
 Variant HHVM_FUNCTION(curl_multi_getcontent, const Resource& ch) {
   CHECK_RESOURCE(curl);
   return curl->getContents();
@@ -1483,6 +1497,7 @@ struct CurlExtension final : Extension {
     HHVM_FE(curl_multi_remove_handle);
     HHVM_FE(curl_multi_exec);
     HHVM_FE(curl_multi_select);
+    HHVM_FE(curl_multi_await);
     HHVM_FE(curl_multi_getcontent);
     HHVM_FE(curl_multi_setopt);
     HHVM_FE(fb_curl_multi_fdset);

hphp/runtime/ext/curl/ext_curl.php

@@ -280,7 +280,7 @@ function curl_multi_select(resource $mh,
  *
  * @param $mh - A cURL multi handle returned from
  *              [`curl_multi_init`](http://php.net/manual/en/function.curl-multi-init.php).
- * @param $timeout - The time (in seconds) to wait for a response indicating some activity.
+ * @param $timeout - The time to wait for a response indicating some activity.
  *
  * @return Awaitable<int> - An `Awaitable` representing the `int` result of the
  *                          activity. If returned `int` is positive, that
@@ -291,21 +291,12 @@ function curl_multi_select(resource $mh,
  *
  * @guide /hack/async/introduction
  * @guide /hack/async/extensions
+ *
+ * See curl_exec() wrt NoFCallBuiltin.
  */
-async function curl_multi_await(
-  resource $mh,
-  float $timeout = 1.0,
-): Awaitable<int> {
-  $finish_by = \microtime(true) + $timeout;
-  do {
-    $result = \curl_multi_select($mh, 0.0);
-    if ($result !== 0) {
-      return $result;
-    }
-    await \HH\Asio\later();
-  } while (\microtime(true) < $finish_by);
-  return 0;
-}
+<<__Native("NoFCallBuiltin")>>
+function curl_multi_await(resource $mh,
+                          float $timeout = 1.0): Awaitable<int>;
 
 /**
  * Wait for activity on any curl_multi connection