Mojo: Rewrite comments in mojo/public/c/environment/async_waiter.h.

mojo/public/c/environment/async_waiter.h

Index: mojo/public/c/environment/async_waiter.h
diff --git a/mojo/public/c/environment/async_waiter.h b/mojo/public/c/environment/async_waiter.h
index 1eb06317d70f25b4a4ec357bb31771659f8ec9d9..376594abed03b559448661caac0f14d6d7894d34 100644
--- a/mojo/public/c/environment/async_waiter.h
+++ b/mojo/public/c/environment/async_waiter.h
@@ -11,19 +11,48 @@ typedef uintptr_t MojoAsyncWaitID;
 
 typedef void (*MojoAsyncWaitCallback)(void* closure, MojoResult result);
 
+// Functions for asynchronously waiting (and cancelling asynchronous waits) on a
+// handle.
+//
+// Thread-safety:
+//   - |CancelWait(wait_id)| may only be called on the same thread as the
+//     |AsyncWait()| that provided |wait_id| was called on.
+//   - A given |MojoAsyncWaiter|'s functions may only be called on the thread(s)
+//     that it is defined to be valid on (typically including the thread on
+//     which the |MojoAsyncWaiter| was provided). E.g., a library require

[darin (slow to review), 2014/07/17 20:54:26]

nit: "a library require" -> "a library [may] require" ?

[viettrungluu, 2014/07/17 21:16:59]

Oops, fixed.

+//     initialization with a single |MojoAsyncWaiter| and stipulate that it only
+//     be used on threads on which that |MojoAsyncWaiter| is valid.
+//   - If a |MojoAsyncWaiter| is valid on multiple threads, then its functions
+//     must be thread-safe (subject to the first restriction above).
 struct MojoAsyncWaiter {
-  // Asynchronously call MojoWait on a background thread, and pass the result
-  // of MojoWait to the given MojoAsyncWaitCallback on the current thread.
-  // Returns a non-zero MojoAsyncWaitID that can be used with CancelWait to
-  // stop waiting. This identifier becomes invalid once the callback runs.
+  // Arranges for |callback| to be called on the current thread at some future
+  // when |handle| satisfies |signals| or it is known that it will never satisfy
+  // |signals| (with the same behavior as |MojoWait()|).
+  //
+  // |callback| will not be called in the nested context of |AsyncWait()|, but
+  // only, e.g., from some run loop. |callback| is provided with the |closure|
+  // argument as well as the result of the wait. For each call to |AsyncWait()|,
+  // |callback| will be called at most once.
+  //
+  // |handle| must not be closed or transferred (via |MojoWriteMessage()|; this

[darin (slow to review), 2014/07/17 20:54:26]

why is this a "must"? what's wrong with transferring a handle while another
thread is blocking calling MojoWait on that handle? won't the MojoWait just see
an error, and then everything is fine?

[viettrungluu, 2014/07/17 21:16:59]

a) There's no guarantee that there's even another thread (e.g., the standalone
impl). In which case there's no notification that that handle is no longer
valid. And of course then the handle value may end up be reused.
b) There's no guarantee that the other thread has started waiting either. You
can't really synchronize on another thread starting to wait in a reasonable way
(plus it'd make AsyncWait() slow).

+  // is equivalent to closing the handle) until either the callback has been
+  // executed or the async wait has been cancelled using the returned (nonzero)
+  // |MojoAsyncWaitID| (see |CancelWait()|). Note that once the callback has
+  // been called, the returned |MojoAsyncWaitID| becomes invalid.
   MojoAsyncWaitID (*AsyncWait)(MojoHandle handle,
                                MojoHandleSignals signals,
                                MojoDeadline deadline,
                                MojoAsyncWaitCallback callback,
                                void* closure);
 
-  // Cancel an existing call to AsyncWait with the given MojoAsyncWaitID. The
-  // corresponding MojoAsyncWaitCallback will not be called in this case.
+  // Cancels an outstanding async wait (specified by |wait_id|) initiated by
+  // |AsyncWait()|. This may only be called from the same thread on which the
+  // corresponding |AsyncWait()| was called, and may only be called if the
+  // callback to |AsyncWait()| has not been called.
+  //
+  // Once this has been called, the callback provided to |AsyncWait()| will not
+  // be called. Moreover, it is then immediately safe to close or transfer the

[darin (slow to review), 2014/07/17 20:54:26]

I don't understand the "safety" argument here.

[viettrungluu, 2014/07/17 21:16:59]

This is really specifying a requirement on CancelWait(). I.e., an implementation
that posts a task to another thread and returns immediately is not compliant.

+  // handle provided to |AsyncWait()|.
   void (*CancelWait)(MojoAsyncWaitID wait_id);
 };