Index: mojo/system/core_impl.h |
diff --git a/mojo/system/core_impl.h b/mojo/system/core_impl.h |
index aede29deffb96794b8ce97160e303ddeffe88051..c05ac3b276697eb4d861b1ecdb30a0b1b9a32ec0 100644 |
--- a/mojo/system/core_impl.h |
+++ b/mojo/system/core_impl.h |
@@ -59,8 +59,40 @@ class MOJO_SYSTEM_EXPORT CoreImpl { |
private: |
friend class test::CoreTestBase; |
- typedef base::hash_map<MojoHandle, scoped_refptr<Dispatcher> > |
- HandleTableMap; |
+ // The |busy| member is used only to deal with functions (in particular |
+ // |WriteMessage()|) that want to hold on to a dispatcher and later remove it |
+ // from the handle table, without holding on to the handle table lock. |
+ // |
+ // For example, if |WriteMessage()| is called with a handle to be sent, (under |
+ // the handle table lock) it must first check that that handle is not busy (if |
+ // it is busy, then it fails with |MOJO_RESULT_BUSY|) and then marks it as |
+ // busy. To avoid deadlock, it should also try to acquire the locks for all |
+ // the dispatchers for the handles that it is sending (and fail with |
+ // |MOJO_RESULT_BUSY| if the attempt fails). At this point, it can release the |
+ // handle table lock. |
+ // |
+ // If |Close()| is simultaneously called on that handle, it too checks if the |
+ // handle is marked busy. If it is, it fails (with |MOJO_RESULT_BUSY|). This |
+ // prevents |WriteMessage()| from sending a handle that has been closed (or |
+ // learning about this too late). |
+ // |
+ // TODO(vtl): Move this implementation note. |
+ // To properly cancel waiters and avoid other races, |WriteMessage()| does not |
+ // transfer dispatchers from one handle to another, even when sending a |
+ // message in-process. Instead, it must transfer the "contents" of the |
+ // dispatcher to a new dispatcher, and then close the old dispatcher. If this |
+ // isn't done, in the in-process case, calls on the old handle may complete |
+ // after the the message has been received and a new handle created (and |
+ // possibly even after calls have been made on the new handle). |
+ struct HandleTableEntry { |
+ HandleTableEntry(); |
+ explicit HandleTableEntry(const scoped_refptr<Dispatcher>& dispatcher); |
+ ~HandleTableEntry(); |
+ |
+ scoped_refptr<Dispatcher> dispatcher; |
+ bool busy; |
+ }; |
+ typedef base::hash_map<MojoHandle, HandleTableEntry> HandleTableMap; |
CoreImpl(); |
~CoreImpl(); |
@@ -72,7 +104,7 @@ class MOJO_SYSTEM_EXPORT CoreImpl { |
// Assigns a new handle for the given dispatcher (which must be valid); |
// returns |MOJO_HANDLE_INVALID| on failure (due to hitting resource limits). |
// Must be called under |handle_table_lock_|. |
- MojoHandle AddDispatcherNoLock(scoped_refptr<Dispatcher> dispatcher); |
+ MojoHandle AddDispatcherNoLock(const scoped_refptr<Dispatcher>& dispatcher); |
// Internal implementation of |Wait()| and |WaitMany()|; doesn't do basic |
// validation of arguments. |