hyper: remove `hyptransfer->endtask`

`Curl_hyper_stream` needs to distinguish between two kinds of
`HYPER_TASK_EMPTY` tasks: (a) the `foreach` tasks it creates itself, and
(b) background tasks that hyper produces. It does this by recording the
address of any `foreach` task in `hyptransfer->endtask` before pushing
it into the executor, and then comparing that against the address of
tasks later polled out of the executor.

This works right now, but there is no guarantee from hyper that the
addresses are stable. `hyper_executor_push` says "The executor takes
ownership of the task, which should not be accessed again unless
returned back to the user with `hyper_executor_poll`". That wording is a
bit ambiguous but with my Rust programmer's hat on I read it as meaning
the task returned with `hyper_executor_poll` may be conceptually the
same as a task that was pushed, but that there are no other guarantees
and comparing addresses is a bad idea.

This commit instead uses `hyper_task_set_userdata` to mark the `foreach`
task with a `USERDATA_RESP_BODY` value which can then be checked for,
removing the need for `hyptransfer->endtask`. This makes the code look
more like that hyper C API examples, which use userdata for every task
and never look at task addresses.

Closes #11779

diff --git a/lib/c-hyper.c b/lib/c-hyper.c
index 05080fa9ac838c13c91bb293214808be2c6ea9cf..61ca29a3acaeef95a7433eb881953727853dff8e 100644 (file)
--- a/lib/c-hyper.c
+++ b/lib/c-hyper.c
@@ -61,6 +61,11 @@
 #include "curl_memory.h"
 #include "memdebug.h"
 
+typedef enum {
+    USERDATA_NOT_SET = 0, /* for tasks with no userdata set; must be zero */
+    USERDATA_RESP_BODY
+} userdata_t;
+
 size_t Curl_hyper_recv(void *userp, hyper_context *ctx,
                        uint8_t *buf, size_t buflen)
 {
@@ -350,7 +355,6 @@ CURLcode Curl_hyper_stream(struct Curl_easy *data,
   struct hyptransfer *h = &data->hyp;
   hyper_task *task;
   hyper_task *foreach;
-  hyper_error *hypererr = NULL;
   const uint8_t *reasonp;
   size_t reason_len;
   CURLcode result = CURLE_OK;
@@ -393,19 +397,9 @@ CURLcode Curl_hyper_stream(struct Curl_easy *data,
       break;
     }
     t = hyper_task_type(task);
-    switch(t) {
-    case HYPER_TASK_ERROR:
-      hypererr = hyper_task_value(task);
-      break;
-    case HYPER_TASK_RESPONSE:
-      resp = hyper_task_value(task);
-      break;
-    default:
-      break;
-    }
-    hyper_task_free(task);
-
     if(t == HYPER_TASK_ERROR) {
+      hyper_error *hypererr = hyper_task_value(task);
+      hyper_task_free(task);
       if(data->state.hresult) {
         /* override Hyper's view, might not even be an error */
         result = data->state.hresult;
@@ -441,23 +435,30 @@ CURLcode Curl_hyper_stream(struct Curl_easy *data,
       hyper_error_free(hypererr);
       break;
     }
-    else if(h->endtask == task) {
-      /* end of transfer, forget the task handled, we might get a
-       * new one with the same address in the future. */
-      *done = TRUE;
-      h->endtask = NULL;
-      infof(data, "hyperstream is done");
-      if(!k->bodywrites) {
-        /* hyper doesn't always call the body write callback */
-        bool stilldone;
-        result = Curl_http_firstwrite(data, data->conn, &stilldone);
+    else if(t == HYPER_TASK_EMPTY) {
+      void *userdata = hyper_task_userdata(task);
+      hyper_task_free(task);
+      if((userdata_t)userdata == USERDATA_RESP_BODY) {
+        /* end of transfer */
+        *done = TRUE;
+        infof(data, "hyperstream is done");
+        if(!k->bodywrites) {
+          /* hyper doesn't always call the body write callback */
+          bool stilldone;
+          result = Curl_http_firstwrite(data, data->conn, &stilldone);
+        }
+        break;
+      }
+      else {
+        /* A background task for hyper; ignore */
+        continue;
       }
-      break;
-    }
-    else if(t != HYPER_TASK_RESPONSE) {
-      continue;
     }
-    /* HYPER_TASK_RESPONSE */
+
+    DEBUGASSERT(HYPER_TASK_RESPONSE);
+
+    resp = hyper_task_value(task);
+    hyper_task_free(task);
 
     *didwhat = KEEP_RECV;
     if(!resp) {
@@ -537,13 +538,12 @@ CURLcode Curl_hyper_stream(struct Curl_easy *data,
       result = CURLE_OUT_OF_MEMORY;
       break;
     }
-    DEBUGASSERT(hyper_task_type(foreach) == HYPER_TASK_EMPTY);
+    hyper_task_set_userdata(foreach, (void *)USERDATA_RESP_BODY);
     if(HYPERE_OK != hyper_executor_push(h->exec, foreach)) {
       failf(data, "Couldn't hyper_executor_push the body-foreach");
       result = CURLE_OUT_OF_MEMORY;
       break;
     }
-    h->endtask = foreach;
 
     hyper_response_free(resp);
     resp = NULL;

diff --git a/lib/c-hyper.h b/lib/c-hyper.h
index 4218cda75fe91c7df3120e0a49c4e6e6e528fb4b..0c7de90b70b721be379a14b94853ba853355aba3 100644 (file)
--- a/lib/c-hyper.h
+++ b/lib/c-hyper.h
@@ -34,7 +34,6 @@ struct hyptransfer {
   hyper_waker *write_waker;
   hyper_waker *read_waker;
   const hyper_executor *exec;
-  hyper_task *endtask;
   hyper_waker *exp100_waker;
   hyper_waker *send_body_waker;
 };