Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2781443004:
Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique (Closed)
DescriptionConvert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique
NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is
accessed from multiple threads. Though it's used safely behind a lock,
it confuses an upcoming threading assertion in RefCounted.
This CL removes its ref count, and has NaClIPCAdapter handle it in
std::unique_ptr as C++11 STL container can hold non-copyable values and
the ref count is no longer needed.
BUG=688072
Review-Url: https://codereview.chromium.org/2781443004
Cr-Commit-Position: refs/heads/master@{#467000}
Committed: https://chromium.googlesource.com/chromium/src/+/1920c68e77540a5a35e52fba0860f1b8a6262e57
Patch Set 1 #Patch Set 2 : s/scoped_refptr/std::unique_ptr/ #
Messages
Total messages: 26 (16 generated)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Avoid unneeded ref count bump of NaClIPCAdapter::RewrittenMessage ========== to ========== Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is accessed from multiple threads. Though it's used safely behind a lock, it confuses an upcoming threading assertion in RefCounted. This CL removes its ref count, and has NaClIPCAdapter handle it in std::unique_ptr as C++11 STL container can hold non-copyable values and the ref count is no longer needed. ==========
tzik@chromium.org changed reviewers: + bradnelson@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is accessed from multiple threads. Though it's used safely behind a lock, it confuses an upcoming threading assertion in RefCounted. This CL removes its ref count, and has NaClIPCAdapter handle it in std::unique_ptr as C++11 STL container can hold non-copyable values and the ref count is no longer needed. ========== to ========== Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is accessed from multiple threads. Though it's used safely behind a lock, it confuses an upcoming threading assertion in RefCounted. This CL removes its ref count, and has NaClIPCAdapter handle it in std::unique_ptr as C++11 STL container can hold non-copyable values and the ref count is no longer needed. BUG=688072 ==========
Ping. Any concern or opinion to this?
tzik@chromium.org changed reviewers: + dschuff@chromium.org
+dschuff. Could you take a look to this?
dschuff@chromium.org changed reviewers: + bbudge@chromium.org
I think this looks ok. +bbudge in case I missed something.
lgtm Nice cleanup!
The CQ bit was checked by tzik@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by dschuff@chromium.org
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493134640169760,
"parent_rev": "a15eba14554af2a846bf977301dee043a0aa54a1", "commit_rev":
"1920c68e77540a5a35e52fba0860f1b8a6262e57"}
Message was sent while issue was closed.
Description was changed from ========== Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is accessed from multiple threads. Though it's used safely behind a lock, it confuses an upcoming threading assertion in RefCounted. This CL removes its ref count, and has NaClIPCAdapter handle it in std::unique_ptr as C++11 STL container can hold non-copyable values and the ref count is no longer needed. BUG=688072 ========== to ========== Convert NaClIPCAdapter::RewrittenMessage ownership management from shared to unique NaClIPCAdapter::RewrittenMessage was a non-thread-safe ref count and is accessed from multiple threads. Though it's used safely behind a lock, it confuses an upcoming threading assertion in RefCounted. This CL removes its ref count, and has NaClIPCAdapter handle it in std::unique_ptr as C++11 STL container can hold non-copyable values and the ref count is no longer needed. BUG=688072 Review-Url: https://codereview.chromium.org/2781443004 Cr-Commit-Position: refs/heads/master@{#467000} Committed: https://chromium.googlesource.com/chromium/src/+/1920c68e77540a5a35e52fba0860... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/1920c68e77540a5a35e52fba0860... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
