Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(489)

Unified Diff: mojo/system/core_impl.h

Issue 67413003: Mojo: Implement plumbing to support passing handles over MessagePipes. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: old chunk mismatch Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: mojo/system/core_impl.h
diff --git a/mojo/system/core_impl.h b/mojo/system/core_impl.h
index aede29deffb96794b8ce97160e303ddeffe88051..382f973f6d22828b24fce879f5d8ac9abf6bd735 100644
--- a/mojo/system/core_impl.h
+++ b/mojo/system/core_impl.h
@@ -59,8 +59,41 @@ 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 wants to hold onto a dispatcher and later remove it
darin (slow to review) 2013/11/11 21:22:21 nit: wants -> want, onto -> on to
viettrungluu 2013/11/11 22:40:26 Done.
+ // 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 create transfer the "contents" of the
darin (slow to review) 2013/11/11 21:22:21 nit: "create transfer"
viettrungluu 2013/11/11 22:40:26 Done.
+ // 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;
+
darin (slow to review) 2013/11/11 21:22:21 nit: extra new line
viettrungluu 2013/11/11 22:40:26 Done.
CoreImpl();
~CoreImpl();
@@ -72,7 +105,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.
« no previous file with comments | « mojo/public/system/core.h ('k') | mojo/system/core_impl.cc » ('j') | mojo/system/core_impl.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698