Re: Issue 143950 in chromium: Chrome_Mac: rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock

[chro...@googlecode.com]

Comment #2 on issue 143950 by dhar...@google.com: Chrome_Mac:
rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950

I see only one crash in M22 - 1229.6 and that too with different stack
trace.

https://crash.corp.google.com/reportdetail?reportid=01d17285618d20dc#crashing_thread

[chro...@googlecode.com]

Comment #4 on issue 143950 by tha...@chromium.org: Chrome_Mac:

rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950

https://chromiumcodereview.appspot.com/10828424/

[chro...@googlecode.com]

Comment #5 on issue 143950 by bugdro...@chromium.org: Chrome_Mac:
rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950#c5

The following revision refers to this bug:
http://src.chromium.org/viewvc/chrome?view=rev&revision=152659

------------------------------------------------------------------------
r152659 | tha...@chromium.org | 2012-08-21T22:53:54.956035Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/rlz/lib/rlz_lib_test.cc?r1=152659&r2=152658&pathrev=152659
M
http://src.chromium.org/viewvc/chrome/trunk/src/rlz/lib/rlz_value_store.h?r1=152659&r2=152658&pathrev=152659
M
http://src.chromium.org/viewvc/chrome/trunk/src/rlz/mac/lib/rlz_value_store_mac.mm?r1=152659&r2=152658&pathrev=152659

mac/rlz: Remove an incorrect check

As a comment in the ScopedRlzValueStoreLock destructor explains:

// Check that "store_ set" => "file_lock acquired". The converse isn't
true,
// for example if the rlz data file can't be read.

So don't CHECK when that happens, instead treat it like lock acquisition
failures:
Silently drop events when that happens. I added a unit test for this
scenario.

Also pass O_RDWR to open(), as posix requires one of O_READ, O_WRITE, or
O_RDWR.

BUG=143950


Review URL: https://chromiumcodereview.appspot.com/10828424
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Status: Fixed
Labels: -Mstone-23 Mstone-22 Merge-Requested

Comment #6 on issue 143950 by tha...@chromium.org: Chrome_Mac:

rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950

This should be better, let me know if it comes back.

If the patch does help, we probably want to merge it to m22, since that has
the same code as m23.

[chro...@googlecode.com]

Updates:
Status: Fixed
Labels: -Merge-Approved Merge-Merged

Comment #10 on issue 143950 by tha...@chromium.org: Chrome_Mac:

rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950

(No comment was entered for this change.)

[chro...@googlecode.com]

Comment #12 on issue 143950 by bugdro...@chromium.org: Chrome_Mac:
rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950#c12

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=152888

------------------------------------------------------------------------
r152888 | tha...@chromium.org | 2012-08-23T00:41:43.276633Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/branches/1229/src/rlz/lib/rlz_lib_test.cc?r1=152888&r2=152887&pathrev=152888
M
http://src.chromium.org/viewvc/chrome/branches/1229/src/rlz/lib/rlz_value_store.h?r1=152888&r2=152887&pathrev=152888
M
http://src.chromium.org/viewvc/chrome/branches/1229/src/rlz/mac/lib/rlz_value_store_mac.mm?r1=152888&r2=152887&pathrev=152888

Merge 152659 - mac/rlz: Remove an incorrect check

As a comment in the ScopedRlzValueStoreLock destructor explains:

// Check that "store_ set" => "file_lock acquired". The converse isn't
true,
// for example if the rlz data file can't be read.

So don't CHECK when that happens, instead treat it like lock acquisition
failures:
Silently drop events when that happens. I added a unit test for this
scenario.

Also pass O_RDWR to open(), as posix requires one of O_READ, O_WRITE, or
O_RDWR.

BUG=143950


Review URL: https://chromiumcodereview.appspot.com/10828424

TBR=tha...@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10882005
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Labels: merge-merged-1229

Comment #11 on issue 143950 by bugdro...@chromium.org: Chrome_Mac:
rlz_lib::ScopedRlzValueStoreLock::ScopedRlzValueStoreLock
http://code.google.com/p/chromium/issues/detail?id=143950#c11

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=152887

------------------------------------------------------------------------
r152887 | tha...@chromium.org | 2012-08-23T00:40:02.448799Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/branches/1229/src/rlz/mac/lib/rlz_value_store_mac.mm?r1=152887&r2=152886&pathrev=152887

Merge 152616 - mac/rlz: Correctly check for fd existence

Found while looking at this file for http://crbug.com/143950, but
I don't think this causes that bug.

BUG=143950

Review URL: https://chromiumcodereview.appspot.com/10827449

TBR=tha...@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10868033
------------------------------------------------------------------------