docs(ffi): clarify ownership responsibilities in the C API

Curl had two memory leaks in its usage of the hyper C API. I fixed these
in curl/curl#11729. While fixing these, as a
hyper newbie I found a lack of clarity in the docs about where the
responsibilities lie for allocating and deallocating things. I had to
repeatedly look at the `Box::into_raw`/`Box::from_raw` calls in the C
API code to properly understand things.

This commit extends the docs.
- For all functions that allocate things, make it clear which functions
  can be used to subsequently deallocate.
- For all functions that free things, make it clear if there are (or
  aren't) other functions that may have otherwise consumed the thing,
  thus removing the need for explicit freeing.
- Clarify some functions that consume things.

This is the documentation I wished was present when I was fixing the
leaks in curl.

Author: nnethercote

capi/include/hyper.h

@@ -191,6 +191,10 @@ extern "C" {
  - `HYPER_TASK_ERROR`: An error retrieving the data.
  - `HYPER_TASK_EMPTY`: The body has finished streaming data.
 
+ To avoid a memory leak, the task must eventually be consumed by
+ `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+ without subsequently being given back by `hyper_executor_poll`.
+
  This does not consume the `hyper_body *`, so it may be used to again.
  However, it MUST NOT be used or freed until the related task completes.
  */
@@ -200,6 +204,10 @@ hyper_task *hyper_body_data(hyper_body *body);
  Return a task that will poll the body and execute the callback with each
  body chunk that is received.
 
+ To avoid a memory leak, the task must eventually be consumed by
+ `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+ without subsequently being given back by `hyper_executor_poll`.
+
  The `hyper_buf` pointer is only a borrowed reference, it cannot live outside
  the execution of the callback. You must make a copy to retain it.
 
@@ -211,14 +219,20 @@ hyper_task *hyper_body_data(hyper_body *body);
 hyper_task *hyper_body_foreach(hyper_body *body, hyper_body_foreach_callback func, void *userdata);
 
 /*
- Free a `hyper_body *`.
+ Free a body.
+
+ This should only be used if the request isn't consumed by
+ `hyper_body_foreach` or `hyper_request_set_body`.
  */
 void hyper_body_free(hyper_body *body);
 
 /*
  Create a new "empty" body.
 
  If not configured, this body acts as an empty payload.
+
+ To avoid a memory leak, the body must eventually be consumed by
+ `hyper_body_free`, `hyper_body_foreach`, or `hyper_request_set_body`.
  */
 hyper_body *hyper_body_new(void);
 
@@ -265,14 +279,19 @@ const uint8_t *hyper_buf_bytes(const hyper_buf *buf);
  Create a new `hyper_buf *` by copying the provided bytes.
 
  This makes an owned copy of the bytes, so the `buf` argument can be
- freed or changed afterwards.
+ freed (with `hyper_buf_free`) or changed afterwards.
+
+ To avoid a memory leak, the copy must eventually be consumed by
+ `hyper_buf_free`.
 
  This returns `NULL` if allocating a new buffer fails.
  */
 hyper_buf *hyper_buf_copy(const uint8_t *buf, size_t len);
 
 /*
  Free this buffer.
+
+ This should be used for any buffer once it is no longer needed.
  */
 void hyper_buf_free(hyper_buf *buf);
 
@@ -283,6 +302,8 @@ size_t hyper_buf_len(const hyper_buf *buf);
 
 /*
  Free a `hyper_clientconn *`.
+
+ This should be used for any connection once it is no longer needed.
  */
 void hyper_clientconn_free(hyper_clientconn *conn);
 
@@ -291,9 +312,14 @@ void hyper_clientconn_free(hyper_clientconn *conn);
  and options.
 
  Both the `io` and the `options` are consumed in this function call.
+ They should not be used or freed afterwards.
 
- The returned `hyper_task *` must be polled with an executor until the
- handshake completes, at which point the value can be taken.
+ The returned task must be polled with an executor until the handshake
+ completes, at which point the value can be taken.
+
+ To avoid a memory leak, the task must eventually be consumed by
+ `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+ without subsequently being given back by `hyper_executor_poll`.
  */
 hyper_task *hyper_clientconn_handshake(hyper_io *io, hyper_clientconn_options *options);
 
@@ -305,7 +331,10 @@ hyper_task *hyper_clientconn_handshake(hyper_io *io, hyper_clientconn_options *o
 void hyper_clientconn_options_exec(hyper_clientconn_options *opts, const hyper_executor *exec);
 
 /*
- Free a `hyper_clientconn_options *`.
+ Free a set of HTTP clientconn options.
+
+ This should only be used if the options aren't consumed by
+ `hyper_clientconn_handshake`.
  */
 void hyper_clientconn_options_free(hyper_clientconn_options *opts);
 
@@ -328,6 +357,9 @@ hyper_code hyper_clientconn_options_http2(hyper_clientconn_options *opts, int en
 
 /*
  Creates a new set of HTTP clientconn options to be used in a handshake.
+
+ To avoid a memory leak, the options must eventually be consumed by
+ `hyper_clientconn_options_free` or `hyper_clientconn_handshake`.
  */
 hyper_clientconn_options *hyper_clientconn_options_new(void);
 
@@ -349,13 +381,23 @@ void hyper_clientconn_options_set_preserve_header_order(hyper_clientconn_options
 /*
  Send a request on the client connection.
 
+ This consumes the request. You should not use or free the request
+ afterwards.
+
  Returns a task that needs to be polled until it is ready. When ready, the
  task yields a `hyper_response *`.
+
+ To avoid a memory leak, the task must eventually be consumed by
+ `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+ without subsequently being given back by `hyper_executor_poll`.
  */
 hyper_task *hyper_clientconn_send(hyper_clientconn *conn, hyper_request *req);
 
 /*
  Copies a waker out of the task context.
+
+ To avoid a memory leak, the waker must eventually be consumed by
+ `hyper_waker_free` or `hyper_waker_wake`.
  */
 hyper_waker *hyper_context_waker(hyper_context *cx);
 
@@ -366,6 +408,8 @@ hyper_code hyper_error_code(const hyper_error *err);
 
 /*
  Frees a `hyper_error`.
+
+ This should be used for any error once it is no longer needed.
  */
 void hyper_error_free(hyper_error *err);
 
@@ -381,11 +425,16 @@ size_t hyper_error_print(const hyper_error *err, uint8_t *dst, size_t dst_len);
 
 /*
  Frees an executor and any incomplete tasks still part of it.
+
+ This should be used for any executor once it is no longer needed.
  */
 void hyper_executor_free(const hyper_executor *exec);
 
 /*
  Creates a new task executor.
+
+ To avoid a memory leak, the executor must eventually be consumed by
+ `hyper_executor_free`.
  */
 const hyper_executor *hyper_executor_new(void);
 
@@ -395,14 +444,18 @@ const hyper_executor *hyper_executor_new(void);
 
  If ready, returns a task from the executor that has completed.
 
+ To avoid a memory leak, the task must eventually be consumed by
+ `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+ without subsequently being given back by `hyper_executor_poll`.
+
  If there are no ready tasks, this returns `NULL`.
  */
 hyper_task *hyper_executor_poll(const hyper_executor *exec);
 
 /*
  Push a task onto the executor.
 
- The executor takes ownership of the task, it should not be accessed
+ The executor takes ownership of the task, which should not be accessed
  again unless returned back to the user with `hyper_executor_poll`.
  */
 hyper_code hyper_executor_push(const hyper_executor *exec, hyper_task *task);
@@ -443,10 +496,10 @@ hyper_code hyper_headers_set(hyper_headers *headers,
                              size_t value_len);
 
 /*
- Free an unused `hyper_io *`.
+ Free an IO handle.
 
- This is typically only useful if you aren't going to pass ownership
- of the IO handle to hyper, such as with `hyper_clientconn_handshake()`.
+ This should only be used if the request isn't consumed by
+ `hyper_clientconn_handshake`.
  */
 void hyper_io_free(hyper_io *io);
 
@@ -455,6 +508,9 @@ void hyper_io_free(hyper_io *io);
 
  The read and write functions of this transport should be set with
  `hyper_io_set_read` and `hyper_io_set_write`.
+
+ To avoid a memory leak, the IO handle must eventually be consumed by
+ `hyper_io_free` or `hyper_clientconn_handshake`.
  */
 hyper_io *hyper_io_new(void);
 
@@ -503,7 +559,10 @@ void hyper_io_set_userdata(hyper_io *io, void *data);
 void hyper_io_set_write(hyper_io *io, hyper_io_write_callback func);
 
 /*
- Free an HTTP request if not going to send it on a client.
+ Free an HTTP request.
+
+ This should only be used if the request isn't consumed by
+ `hyper_clientconn_send`.
  */
 void hyper_request_free(hyper_request *req);
 
@@ -517,6 +576,9 @@ hyper_headers *hyper_request_headers(hyper_request *req);
 
 /*
  Construct a new HTTP request.
+
+ To avoid a memory leak, the request must eventually be consumed by
+ `hyper_request_free` or `hyper_clientconn_send`.
  */
 hyper_request *hyper_request_new(void);
 
@@ -605,11 +667,16 @@ hyper_code hyper_request_set_version(hyper_request *req, int version);
  Take ownership of the body of this response.
 
  It is safe to free the response even after taking ownership of its body.
+
+ To avoid a memory leak, the body must eventually be consumed by
+ `hyper_body_free`, `hyper_body_foreach`, or `hyper_request_set_body`.
  */
 hyper_body *hyper_response_body(hyper_response *resp);
 
 /*
- Free an HTTP response after using it.
+ Free an HTTP response.
+
+ This should be used for any response once it is no longer needed.
  */
 void hyper_response_free(hyper_response *resp);
 
@@ -662,6 +729,10 @@ int hyper_response_version(const hyper_response *resp);
 
 /*
  Free a task.
+
+ This should only be used if the task isn't consumed by
+ `hyper_clientconn_handshake` or taken ownership of by
+ `hyper_executor_push`.
  */
 void hyper_task_free(hyper_task *task);
 
@@ -690,6 +761,11 @@ void *hyper_task_userdata(hyper_task *task);
  this task.
 
  Use `hyper_task_type` to determine the type of the `void *` return value.
+
+ To avoid a memory leak, a non-empty return value must eventually be
+ consumed by a function appropriate for its type, one of
+ `hyper_error_free`, `hyper_clientconn_free`, `hyper_response_free`, or
+ `hyper_buf_free`.
  */
 void *hyper_task_value(hyper_task *task);
 
@@ -699,7 +775,10 @@ void *hyper_task_value(hyper_task *task);
 const char *hyper_version(void);
 
 /*
- Free a waker that hasn't been woken.
+ Free a waker.
+
+ This should only be used if the request isn't consumed by
+ `hyper_waker_wake`.
  */
 void hyper_waker_free(hyper_waker *waker);
 

src/ffi/body.rs

@@ -32,13 +32,19 @@ ffi_fn! {
     /// Create a new "empty" body.
     ///
     /// If not configured, this body acts as an empty payload.
+    ///
+    /// To avoid a memory leak, the body must eventually be consumed by
+    /// `hyper_body_free`, `hyper_body_foreach`, or `hyper_request_set_body`.
     fn hyper_body_new() -> *mut hyper_body {
         Box::into_raw(Box::new(hyper_body(IncomingBody::ffi())))
     } ?= ptr::null_mut()
 }
 
 ffi_fn! {
-    /// Free a `hyper_body *`.
+    /// Free a body.
+    ///
+    /// This should only be used if the request isn't consumed by
+    /// `hyper_body_foreach` or `hyper_request_set_body`.
     fn hyper_body_free(body: *mut hyper_body) {
         drop(non_null!(Box::from_raw(body) ?= ()));
     }
@@ -53,6 +59,10 @@ ffi_fn! {
     /// - `HYPER_TASK_ERROR`: An error retrieving the data.
     /// - `HYPER_TASK_EMPTY`: The body has finished streaming data.
     ///
+    /// To avoid a memory leak, the task must eventually be consumed by
+    /// `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+    /// without subsequently being given back by `hyper_executor_poll`.
+    ///
     /// This does not consume the `hyper_body *`, so it may be used to again.
     /// However, it MUST NOT be used or freed until the related task completes.
     fn hyper_body_data(body: *mut hyper_body) -> *mut hyper_task {
@@ -81,6 +91,10 @@ ffi_fn! {
     /// Return a task that will poll the body and execute the callback with each
     /// body chunk that is received.
     ///
+    /// To avoid a memory leak, the task must eventually be consumed by
+    /// `hyper_task_free`, or taken ownership of by `hyper_executor_push`
+    /// without subsequently being given back by `hyper_executor_poll`.
+    ///
     /// The `hyper_buf` pointer is only a borrowed reference, it cannot live outside
     /// the execution of the callback. You must make a copy to retain it.
     ///
@@ -194,7 +208,10 @@ ffi_fn! {
     /// Create a new `hyper_buf *` by copying the provided bytes.
     ///
     /// This makes an owned copy of the bytes, so the `buf` argument can be
-    /// freed or changed afterwards.
+    /// freed (with `hyper_buf_free`) or changed afterwards.
+    ///
+    /// To avoid a memory leak, the copy must eventually be consumed by
+    /// `hyper_buf_free`.
     ///
     /// This returns `NULL` if allocating a new buffer fails.
     fn hyper_buf_copy(buf: *const u8, len: size_t) -> *mut hyper_buf {
@@ -227,6 +244,8 @@ ffi_fn! {
 
 ffi_fn! {
     /// Free this buffer.
+    ///
+    /// This should be used for any buffer once it is no longer needed.
     fn hyper_buf_free(buf: *mut hyper_buf) {
         drop(unsafe { Box::from_raw(buf) });
     }