unify request object to use uv_queue_work for both corefoundation + curl

3 files changed, 81 insertions, 156 deletions

diff --git a/common/curl_request.cpp b/common/curl_request.cpp
index 7cfe8fcabd..cca50447f4 100644
--- a/common/curl_request.cpp
+++ b/common/curl_request.cpp
@@ -1,6 +1,6 @@
-#include "curl_request.hpp"
 
 #include <llmr/platform/platform.hpp>
+#include <llmr/platform/request.hpp>
 #include <llmr/util/uv.hpp>
 #include <llmr/util/std.hpp>
 
@@ -84,6 +84,18 @@ static CURLSH *curl_share = nullptr;
 // all the time.
 static std::queue<CURL *> curl_handle_cache;
 
+
+class CURLRequest : public llmr::platform::Request {
+public:
+    CURLRequest(const std::string &url,
+                      std::function<void(llmr::platform::Response *)> background_function,
+                      std::function<void()> foreground_callback)
+        : Request(url, background_function, foreground_callback) {}
+
+    CURL *curl = nullptr;
+};
+
+
 // Implementation starts here.
 
 // Locks the CURL share handle
@@ -165,14 +177,14 @@ void curl_perform(uv_poll_t *req, int /*status*/, int events) {
 
             // Executes the background_function in a libuv thread pool, and the after_work_cb back
             // in the *main* event loop.
-            uv_queue_work(uv_default_loop(), work, Request::work_cb, Request::after_work_cb);
+            uv_queue_work(uv_default_loop(), work, Request::work_callback, Request::after_work_callback);
 
             CURL *handle = message->easy_handle;
             remove_curl_handle(handle);
 
             // We're setting this to NULL because there might still be shared_ptrs around that could
             // be cancelled.
-            (*req)->curl = nullptr;
+            ((CURLRequest *)req->get())->curl = nullptr;
 
             break;
         }
@@ -310,7 +322,7 @@ void async_add_cb(uv_async_t * /*async*/) {
             handle = curl_easy_init();
         }
 
-        (*req)->curl = handle;
+        ((CURLRequest *)req->get())->curl = handle;
 
         curl_easy_setopt(handle, CURLOPT_PRIVATE, req);
         curl_easy_setopt(handle, CURLOPT_URL, (*req)->url.c_str());
@@ -328,10 +340,10 @@ void async_cancel_cb(uv_async_t * /*async*/) {
         // It is possible that the request has not yet been started, but that it already has been
         // added to the queue for scheduling new requests. In this case, the CURL handle is invalid
         // and we manually mark the Request as cancelled.
-        CURL *handle = (*req)->curl;
+        CURL *handle = ((CURLRequest *)req->get())->curl;
         if (handle && !(*req)->cancelled) {
             remove_curl_handle(handle);
-            (*req)->curl = nullptr;
+            ((CURLRequest *)req->get())->curl = nullptr;
         }
         (*req)->cancelled = true;
 
@@ -350,61 +362,15 @@ void thread_init_cb() {
 }
 } // end namespace request
 } // end namespace platform
-} // end namespace llmr
-
-using namespace llmr::platform;
-
-Request::Request(const std::string &url,
-                 std::function<void(Response *)> background_function,
-                 std::function<void()> foreground_callback)
-    : url(url),
-      background_function(background_function),
-      foreground_callback(foreground_callback) {
-
-    // Add a check handle without a callback to keep the default loop running.
-    // We don't have a real handler attached to the default loop right from the
-    // beginning, because we're using asynchronous messaging to perform the actual
-    // CURL request in the request thread. Only after the request is complete, we
-    // create an actual work request that is attached to the default loop.
-    check = new uv_check_t();
-    uv_check_init(uv_default_loop(), check);
-    uv_check_start(check, [](uv_check_t *) {});
-}
-
-Request::~Request() {
-    // We need to remove our no-op handle again to allow the main event loop to exit.
-    uv_check_stop(check);
-}
-
-void Request::work_cb(uv_work_t *work) {
-    std::shared_ptr<Request> &req = *static_cast<std::shared_ptr<Request> *>(work->data);
-    req->background_function(req->res.get());
-}
-
-// This callback is executed in the main loop.
-void Request::after_work_cb(uv_work_t *work, int /*status*/) {
-    std::shared_ptr<Request> &req = *static_cast<std::shared_ptr<Request> *>(work->data);
-
-    req->foreground_callback();
-
-    // This finally deletes the *pointer* to the shared pointer we've been holding on since we
-    // pushed it on the add_queue on request creation.
-    delete static_cast<std::shared_ptr<Request> *>(work->data);
-}
-
 
 
-
-
-namespace llmr {
-
 std::shared_ptr<platform::Request>
 platform::request_http(const std::string &url,
                        std::function<void(Response *)> background_function,
                        std::function<void()> foreground_callback) {
     using namespace request;
     init_thread_once(thread_init_cb);
-    std::shared_ptr<Request> req = std::make_shared<Request>(url, background_function, foreground_callback);
+    std::shared_ptr<CURLRequest> req = std::make_shared<CURLRequest>(url, background_function, foreground_callback);
 
     // Note that we are creating a new shared_ptr pointer(!) because the lockless queue can't store
     // objects with nontrivial destructors. We have to make absolutely sure that we manually delete
@@ -427,4 +393,4 @@ void platform::cancel_request_http(const std::shared_ptr<Request> &req) {
         uv_async_send(&async_cancel);
     }
 }
-}
\ No newline at end of file
+} // end namespace llmr
diff --git a/common/curl_request.hpp b/common/curl_request.hpp
deleted file mode 100644
index b85782500a..0000000000
--- a/common/curl_request.hpp
+++ /dev/null
@@ -1,45 +0,0 @@
-#ifndef LLMR_COMMON_CURL_REQUEST
-#define LLMR_COMMON_CURL_REQUEST
-
-#include <string>
-#include <functional>
-#include <memory>
-
-#include <llmr/util/noncopyable.hpp>
-
-// Forward definition.
-typedef void CURL;
-typedef struct uv_work_s uv_work_t;
-typedef struct uv_check_s uv_check_t;
-
-namespace llmr {
-namespace platform {
-
-struct Response;
-
-class Request : public std::enable_shared_from_this<Request>, private util::noncopyable {
-public:
-    Request(const std::string &url,
-            std::function<void(Response *)> background_function,
-            std::function<void()> foreground_callback);
-    ~Request();
-
-public:
-    static void work_cb(uv_work_t *work);
-    static void after_work_cb(uv_work_t *work, int status);
-
-
-public:
-    const std::string url;
-    const std::function<void(Response *)> background_function;
-    const std::function<void()> foreground_callback;
-    std::unique_ptr<Response> res;
-    CURL *curl = nullptr;
-    bool cancelled = false;
-
-    uv_check_t *check = nullptr;
-};
-}
-}
-
-#endif
diff --git a/common/foundation_request.mm b/common/foundation_request.mm
index 47fffd5c6e..2bd651808a 100644
--- a/common/foundation_request.mm
+++ b/common/foundation_request.mm
@@ -9,25 +9,21 @@
 #include <memory>
 #include <string>
 #include <functional>
+#include <llmr/platform/request.hpp>
 #include <llmr/platform/platform.hpp>
+#include <llmr/util/std.hpp>
 #include <uv.h>
 
-
 uv_once_t request_initialize = UV_ONCE_INIT;
-dispatch_queue_t queue = nullptr;
 NSURLSession *session = nullptr;
 
-
 #if TARGET_OS_IPHONE
 std::atomic<int> active_tasks;
 #endif
 
 void request_initialize_cb() {
-    NSString *queueName = [[[NSBundle mainBundle] bundleIdentifier] stringByAppendingString:@".parsing"];
-
-    queue = dispatch_queue_create([queueName UTF8String], DISPATCH_QUEUE_CONCURRENT);
-
-    NSURLSessionConfiguration *sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration];
+    NSURLSessionConfiguration *sessionConfig =
+        [NSURLSessionConfiguration defaultSessionConfiguration];
     sessionConfig.timeoutIntervalForResource = 6;
     sessionConfig.HTTPMaximumConnectionsPerHost = 8;
     sessionConfig.requestCachePolicy = NSURLRequestUseProtocolCachePolicy;
@@ -39,86 +35,94 @@ void request_initialize_cb() {
 #endif
 }
 
-namespace llmr {
-
-class platform::Request {
+// We're using a child class to make sure ARC is working correctly, as well as to add activity
+// indicators on iOS.
+class FoundationRequest : public llmr::platform::Request {
 public:
-    Request(NSURLSessionDataTask *task, const std::string &original_url)
-        : task(task), original_url(original_url) {
-        #if TARGET_OS_IPHONE
-            active_tasks++;
-            dispatch_async(dispatch_get_main_queue(), ^(void) {
-                [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:(active_tasks > 0)];
-            });
-        #endif
+    FoundationRequest(const std::string &url,
+                      std::function<void(llmr::platform::Response *)> background_function,
+                      std::function<void()> foreground_callback)
+        : Request(url, background_function, foreground_callback) {
+#if TARGET_OS_IPHONE
+        active_tasks++;
+        dispatch_async(dispatch_get_main_queue(), ^(void) {
+            [[UIApplication sharedApplication]
+                setNetworkActivityIndicatorVisible:(active_tasks > 0)];
+        });
+#endif
     }
 
+    ~FoundationRequest() {
 #if TARGET_OS_IPHONE
-    ~Request() {
         active_tasks--;
         dispatch_async(dispatch_get_main_queue(), ^(void) {
-            [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:(active_tasks > 0)];
+            [[UIApplication sharedApplication]
+                setNetworkActivityIndicatorVisible:(active_tasks > 0)];
         });
-    }
 #endif
+    }
 
-    NSURLSessionDataTask *task;
-    std::string original_url;
+    NSURLSessionDataTask *task = nullptr;
 };
 
-std::shared_ptr<platform::Request>
-platform::request_http(const std::string &url, std::function<void(Response *)> background_function,
-                       std::function<void()> foreground_callback) {
-
+std::shared_ptr<llmr::platform::Request>
+llmr::platform::request_http(const std::string &url,
+                             std::function<void(Response *)> background_function,
+                             std::function<void()> foreground_callback) {
     uv_once(&request_initialize, request_initialize_cb);
 
-    __block std::shared_ptr<Request> req;
+    std::shared_ptr<FoundationRequest> req =
+        std::make_shared<FoundationRequest>(url, background_function, foreground_callback);
+
+    // Note that we are creating a new shared_ptr pointer(!) to make sure there is at least one
+    // shared_ptr in existence while the NSURLSession is loading our data. We are making sure in the
+    // callback that this pointer gets destroyed again.
+    std::shared_ptr<Request> *req_ptr = new std::shared_ptr<Request>(req);
+
     NSURLSessionDataTask *task = [session
                                   dataTaskWithURL:[NSURL URLWithString:@(url.c_str())]
                                   completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
         if ([error code] == NSURLErrorCancelled) {
-            // We intentionally cancelled this request.
-            // Make sure we clear the shared_ptr to resolve the circular reference. We're referencing
-            // this shared_ptr by value so that the object stays around until this completion handler is
-            // invoked.
-            req.reset();
+            // We intentionally cancelled this request. Make sure we clear the shared_ptr to resolve
+            // the circular reference. We're referencing this shared_ptr by value so that the object
+            // stays around until this completion handler is invoked.
+            delete req_ptr;
 
             return;
         }
 
-        dispatch_async(queue, ^(){
-            Response res;
+        req->res = std::make_unique<Response>();
 
-            if (!error && [response isKindOfClass:[NSHTTPURLResponse class]]) {
-                res.code = [(NSHTTPURLResponse *)response statusCode];
-                res.body = {(const char *)[data bytes], [data length]};
-            } else {
-                res.code = -1;
-                res.error_message = [[error localizedDescription] UTF8String];
-            }
+        if (!error && [response isKindOfClass:[NSHTTPURLResponse class]]) {
+            req->res->code = [(NSHTTPURLResponse *)response statusCode];
+            req->res->body = {(const char *)[data bytes], [data length]};
+        } else {
+            req->res->error_message = [[error localizedDescription] UTF8String];
+        }
 
-            background_function(&res);
+        // We're currently in the request thread. We're going to schedule a uv_work request that
+        // executes the background function in a threadpool, and tell it to call the after callback
+        // back in the main uv loop.
+        uv_work_t *work = new uv_work_t();
 
-            dispatch_async(dispatch_get_main_queue(), ^(void) {
-                // Make sure we clear the shared_ptr to resolve the circular reference. We're referencing
-                // this shared_ptr by value so that the object stays around until this completion handler is
-                // invoked.
-                req.reset();
+        // We're passing on the pointer we created to the work structure. It is going to be deleted
+        // in the after_work_cb;
+        work->data = req_ptr;
 
-                foreground_callback();
-            });
-        });
+        // Executes the background_function in a libuv thread pool, and the after_work_cb back in
+        // the *main* event loop.
+        uv_queue_work(uv_default_loop(), work, Request::work_callback,
+                      Request::after_work_callback);
     }];
 
-    req = std::make_shared<Request>(task, url);
+    req->task = task;
+
     [task resume];
     return req;
 }
 
-void platform::cancel_request_http(const std::shared_ptr<Request> &req) {
+void llmr::platform::cancel_request_http(const std::shared_ptr<Request> &req) {
     if (req) {
-        [req->task cancel];
+        [((FoundationRequest *)(req.get()))->task cancel];
     }
 }
-
-}