Issue 147052: Fix a race condition in directory_watcher_inotify.cc: release lock before sig...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 147052: Fix a race condition in directory_watcher_inotify.cc: release lock before sig... (Closed)

Created:
11 years, 6 months ago by Paweł Hajdan Jr.

Modified:
9 years, 7 months ago

Reviewers:
Lei Zhang, Evan Martin, the_wrong_timurrrr

CC:
chromium-reviews_googlegroups.com

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

Fix a race condition in directory_watcher_inotify.cc: release lock before signaling the event dtor waits on. http://crbug.com/15055 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=19151

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Created: 11 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+24 lines, -16 lines)			Patch
	M	base/directory_watcher_inotify.cc	View	1	1 chunk	+24 lines, -16 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Paweł Hajdan Jr.

Posting first review from Mountain View! But it's a bit speculative. I don't have Linux ...

11 years, 6 months ago (2009-06-23 22:54:45 UTC) #1

the_wrong_timurrrr

On 2009/06/23 22:54:45, Paweł Hajdan Jr. wrote: > Posting first review from Mountain View! But ...

11 years, 6 months ago (2009-06-24 08:53:53 UTC) #2

Evan Martin

11 years, 6 months ago (2009-06-24 17:51:44 UTC) #4

LGTM

http://codereview.chromium.org/147052/diff/1/2
File base/directory_watcher_inotify.cc (right):

http://codereview.chromium.org/147052/diff/1/2#newcode402
Line 402: // We have to release the lock before signaling
recursive_setup_finished_
This comment will only make sense if you know what the old code is (that kept
the lock for the duration of the function).  It took me a sec to realize what
AutoLock had to do with "release the lock before".

Could you add something to this comment mentioning that you're limiting the
scope of the AutoLock?

Expand Messages | Collapse Messages