New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 821030 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 820540
issue 821067
issue 824144
issue 830907



Sign in to add a comment

modifying a null mojo::ScopedDataPipeProducerHandle doesn't result in an error

Project Member Reported by xunji...@chromium.org, Mar 12 2018

Issue description

I have a mojo::ScopedDataPipeProducerHandle handle_. At some point, I did std::move(handle_), so handle_ is null. Then I did handle_->EndReadData() on that null handle. Mojo didn't give me any error.


rockot@ suggested that this might be unintentional and we can add a DCHECK in https://cs.chromium.org/chromium/src/mojo/public/cpp/system/handle.h?rcl=44d54dc0c98f03c1fc731eaacd3a9497ef219292&l=97



 
Adding a DCHECK on is_valid() correctly catches the error. Let me see if this breaks any existing tests.
Blockedon: 821067
Cc: mmenke@chromium.org
Not surprisingly, some tests are failing.

CQ run is at https://chromium-review.googlesource.com/c/chromium/src/+/957918.

I will take a look at the tests.

Blockedon: 824144
Blockedon: 820540
Blockedon: 830907
Owner: xunji...@chromium.org
Status: Started (was: Untriaged)
All blocking bugs have been fixed. The DCHECK CL (https://chromium-review.googlesource.com/c/chromium/src/+/957918) is cleared to land.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/258b4c2761f30fc3b93041fc97cdc70fe461f217

commit 258b4c2761f30fc3b93041fc97cdc70fe461f217
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Apr 13 02:03:01 2018

Add a DCHECK in mojo/public/cpp/system/handle.h

Bug:  821030 
Change-Id: Iec0c32ba9cc7cb0335e7c320179e2689ab59bba6
Reviewed-on: https://chromium-review.googlesource.com/957918
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550477}
[modify] https://crrev.com/258b4c2761f30fc3b93041fc97cdc70fe461f217/content/browser/renderer_host/clipboard_host_impl_unittest.cc
[modify] https://crrev.com/258b4c2761f30fc3b93041fc97cdc70fe461f217/mojo/public/cpp/system/handle.h

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/258b4c2761f30fc3b93041fc97cdc70fe461f217

commit 258b4c2761f30fc3b93041fc97cdc70fe461f217
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Apr 13 02:03:01 2018

Add a DCHECK in mojo/public/cpp/system/handle.h

Bug:  821030 
Change-Id: Iec0c32ba9cc7cb0335e7c320179e2689ab59bba6
Reviewed-on: https://chromium-review.googlesource.com/957918
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550477}
[modify] https://crrev.com/258b4c2761f30fc3b93041fc97cdc70fe461f217/content/browser/renderer_host/clipboard_host_impl_unittest.cc
[modify] https://crrev.com/258b4c2761f30fc3b93041fc97cdc70fe461f217/mojo/public/cpp/system/handle.h

Sign in to add a comment