Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(738)

Issue 6462081: code review 6462081: os: fix data race on Process.done (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 7 months ago by dfc
Modified:
11 years, 7 months ago
Reviewers:
CC:
dvyukov, r, brainman, minux1, golang-dev
Visibility:
Public.

Description

os: fix data race on Process.done Fixes issue 3969.

Patch Set 1 #

Patch Set 2 : diff -r 7c5f4dad8fae https://code.google.com/p/go #

Patch Set 3 : diff -r 7c5f4dad8fae https://code.google.com/p/go #

Patch Set 4 : diff -r 7c5f4dad8fae https://code.google.com/p/go #

Total comments: 8

Patch Set 5 : diff -r 7c5f4dad8fae https://code.google.com/p/go #

Patch Set 6 : diff -r 7c5f4dad8fae https://code.google.com/p/go #

Patch Set 7 : diff -r baad5c0949fd https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -7 lines) Patch
M src/pkg/os/exec.go View 1 2 3 4 5 3 chunks +10 lines, -1 line 0 comments Download
M src/pkg/os/exec_plan9.go View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/os/exec_unix.go View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/os/exec_windows.go View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 10
dvyukov
Other than that, LGTM http://codereview.appspot.com/6462081/diff/7001/src/pkg/os/exec.go File src/pkg/os/exec.go (right): http://codereview.appspot.com/6462081/diff/7001/src/pkg/os/exec.go#newcode17 src/pkg/os/exec.go:17: done uint32 // atomic bool, ...
11 years, 7 months ago (2012-08-18 12:58:40 UTC) #1
dfc
PTAL. Personally I prefer the channel version, but as os already imports sync/atomic, I have ...
11 years, 7 months ago (2012-08-19 04:08:18 UTC) #2
dfc
Hello dvyukov@google.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
11 years, 7 months ago (2012-08-19 04:08:36 UTC) #3
dfc
Hello dvyukov@google.com (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 7 months ago (2012-08-19 06:16:40 UTC) #4
dvyukov
LGTM
11 years, 7 months ago (2012-08-19 07:53:12 UTC) #5
r
LGTM
11 years, 7 months ago (2012-08-19 16:13:18 UTC) #6
dfc
On 2012/08/19 16:13:18, r wrote: > LGTM + brainman / minux Thanks. Could I get ...
11 years, 7 months ago (2012-08-20 04:47:47 UTC) #7
brainman
LGTM
11 years, 7 months ago (2012-08-20 06:14:39 UTC) #8
minux1
LGTM.
11 years, 7 months ago (2012-08-20 12:35:07 UTC) #9
dfc
11 years, 7 months ago (2012-08-21 00:41:50 UTC) #10
*** Submitted as http://code.google.com/p/go/source/detail?r=4fc1c61c3c2c ***

os: fix data race on Process.done

Fixes issue 3969.

R=dvyukov, r, alex.brainman, minux.ma
CC=golang-dev
http://codereview.appspot.com/6462081
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b