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

Side by Side Diff: ipc/mojo/ipc_message_pipe_reader.h

Issue 1318453002: Fix races with MessagePipeReader due to the Mojo IPC channel being thread-safe. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add bug reference. Created 5 years, 4 months 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 unified diff | Download patch
« no previous file with comments | « no previous file | ipc/mojo/ipc_message_pipe_reader.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #ifndef IPC_IPC_MESSAGE_PIPE_READER_H_ 5 #ifndef IPC_IPC_MESSAGE_PIPE_READER_H_
6 #define IPC_IPC_MESSAGE_PIPE_READER_H_ 6 #define IPC_IPC_MESSAGE_PIPE_READER_H_
7 7
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/atomicops.h"
10 #include "base/compiler_specific.h" 11 #include "base/compiler_specific.h"
11 #include "base/memory/scoped_ptr.h" 12 #include "base/memory/scoped_ptr.h"
13 #include "base/threading/thread_checker.h"
12 #include "ipc/ipc_message.h" 14 #include "ipc/ipc_message.h"
13 #include "third_party/mojo/src/mojo/public/c/environment/async_waiter.h" 15 #include "third_party/mojo/src/mojo/public/c/environment/async_waiter.h"
14 #include "third_party/mojo/src/mojo/public/cpp/system/core.h" 16 #include "third_party/mojo/src/mojo/public/cpp/system/core.h"
15 17
16 namespace IPC { 18 namespace IPC {
17 namespace internal { 19 namespace internal {
18 20
19 class AsyncHandleWaiter; 21 class AsyncHandleWaiter;
20 22
21 // A helper class to handle bytestream directly over mojo::MessagePipe 23 // A helper class to handle bytestream directly over mojo::MessagePipe
22 // in template-method pattern. MessagePipeReader manages the lifetime 24 // in template-method pattern. MessagePipeReader manages the lifetime
23 // of given MessagePipe and participates the event loop, and 25 // of given MessagePipe and participates the event loop, and
24 // read the stream and call the client when it is ready. 26 // read the stream and call the client when it is ready.
25 // 27 //
26 // Each client has to: 28 // Each client has to:
27 // 29 //
28 // * Provide a subclass implemenation of a specific use of a MessagePipe 30 // * Provide a subclass implemenation of a specific use of a MessagePipe
29 // and implement callbacks. 31 // and implement callbacks.
30 // * Create the subclass instance with a MessagePipeHandle. 32 // * Create the subclass instance with a MessagePipeHandle.
31 // The constructor automatically start listening on the pipe. 33 // The constructor automatically start listening on the pipe.
32 // 34 //
33 // MessageReader has to be used in IO thread. It isn't thread-safe. 35 // All functions must be called on the IO thread, except for Send(), which can
36 // be called on any thread. All |Delegate| functions will be called on the IO
37 // thread.
34 // 38 //
35 class MessagePipeReader { 39 class MessagePipeReader {
36 public: 40 public:
37 class Delegate { 41 class Delegate {
38 public: 42 public:
39 virtual void OnMessageReceived(Message& message) = 0; 43 virtual void OnMessageReceived(Message& message) = 0;
40 virtual void OnPipeClosed(MessagePipeReader* reader) = 0; 44 virtual void OnPipeClosed(MessagePipeReader* reader) = 0;
41 virtual void OnPipeError(MessagePipeReader* reader) = 0; 45 virtual void OnPipeError(MessagePipeReader* reader) = 0;
42 }; 46 };
43 47
(...skipping 11 matching lines...) Expand all
55 59
56 void operator()(MessagePipeReader* ptr) const; 60 void operator()(MessagePipeReader* ptr) const;
57 }; 61 };
58 62
59 // Both parameters must be non-null. 63 // Both parameters must be non-null.
60 // Build a reader that reads messages from |handle| and lets |delegate| know. 64 // Build a reader that reads messages from |handle| and lets |delegate| know.
61 // Note that MessagePipeReader doesn't delete |delete|. 65 // Note that MessagePipeReader doesn't delete |delete|.
62 MessagePipeReader(mojo::ScopedMessagePipeHandle handle, Delegate* delegate); 66 MessagePipeReader(mojo::ScopedMessagePipeHandle handle, Delegate* delegate);
63 virtual ~MessagePipeReader(); 67 virtual ~MessagePipeReader();
64 68
65 MojoHandle handle() const { return pipe_.get().value(); } 69 MojoHandle handle() const { return handle_copy_; }
66 70
67 // Returns received bytes. 71 // Returns received bytes.
68 const std::vector<char>& data_buffer() const { 72 const std::vector<char>& data_buffer() const {
69 return data_buffer_; 73 return data_buffer_;
70 } 74 }
71 75
72 // Delegate received handles ownership. The subclass should take the 76 // Delegate received handles ownership. The subclass should take the
73 // ownership over in its OnMessageReceived(). They will leak otherwise. 77 // ownership over in its OnMessageReceived(). They will leak otherwise.
74 void TakeHandleBuffer(std::vector<MojoHandle>* handle_buffer) { 78 void TakeHandleBuffer(std::vector<MojoHandle>* handle_buffer) {
75 handle_buffer_.swap(*handle_buffer); 79 handle_buffer_.swap(*handle_buffer);
(...skipping 17 matching lines...) Expand all
93 void OnPipeClosed(); 97 void OnPipeClosed();
94 void OnPipeError(MojoResult error); 98 void OnPipeError(MojoResult error);
95 99
96 MojoResult ReadMessageBytes(); 100 MojoResult ReadMessageBytes();
97 void PipeIsReady(MojoResult wait_result); 101 void PipeIsReady(MojoResult wait_result);
98 void ReadAvailableMessages(); 102 void ReadAvailableMessages();
99 103
100 std::vector<char> data_buffer_; 104 std::vector<char> data_buffer_;
101 std::vector<MojoHandle> handle_buffer_; 105 std::vector<MojoHandle> handle_buffer_;
102 mojo::ScopedMessagePipeHandle pipe_; 106 mojo::ScopedMessagePipeHandle pipe_;
107 // Constant copy of the message pipe handle. For use by Send(), which can run
108 // concurrently on non-IO threads.
109 // TODO(amistry): This isn't quite right because handles can be re-used and
110 // using this can run into the ABA problem. Currently, this is highly unlikely
111 // because Mojo internally uses an increasing uint32_t as handle values, but
112 // this could change. See crbug.com/524894.
113 const MojoHandle handle_copy_;
103 // |delegate_| and |async_waiter_| are null once the message pipe is closed. 114 // |delegate_| and |async_waiter_| are null once the message pipe is closed.
104 Delegate* delegate_; 115 Delegate* delegate_;
105 scoped_ptr<AsyncHandleWaiter> async_waiter_; 116 scoped_ptr<AsyncHandleWaiter> async_waiter_;
106 MojoResult pending_send_error_; 117 base::subtle::Atomic32 pending_send_error_;
118 base::ThreadChecker thread_checker_;
107 119
108 DISALLOW_COPY_AND_ASSIGN(MessagePipeReader); 120 DISALLOW_COPY_AND_ASSIGN(MessagePipeReader);
109 }; 121 };
110 122
111 } // namespace internal 123 } // namespace internal
112 } // namespace IPC 124 } // namespace IPC
113 125
114 #endif // IPC_IPC_MESSAGE_PIPE_READER_H_ 126 #endif // IPC_IPC_MESSAGE_PIPE_READER_H_
OLDNEW
« no previous file with comments | « no previous file | ipc/mojo/ipc_message_pipe_reader.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698