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

Issue 6855110: code review 6855110: net: fix data races on deadline vars (Closed)

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

Description

net: fix data races on deadline vars Fixes issue 4434.

Patch Set 1 #

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

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

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

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

Total comments: 15

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

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

Total comments: 6

Patch Set 8 : diff -r 88e4ba173900 https://code.google.com/p/go #

Total comments: 2

Patch Set 9 : diff -r a2a0638509dc https://code.google.com/p/go #

Patch Set 10 : diff -r a2a0638509dc https://code.google.com/p/go #

Total comments: 3

Patch Set 11 : diff -r a2a0638509dc https://code.google.com/p/go #

Patch Set 12 : diff -r 73fcac092bd1 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -83 lines) Patch
A src/pkg/net/fd_posix_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +57 lines, -0 lines 0 comments Download
M src/pkg/net/fd_unix.go View 1 2 3 4 5 6 7 8 9 10 13 chunks +61 lines, -53 lines 0 comments Download
M src/pkg/net/fd_windows.go View 1 2 3 4 5 6 6 chunks +38 lines, -10 lines 0 comments Download
M src/pkg/net/sendfile_freebsd.go View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/sendfile_linux.go View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/sock_posix.go View 1 2 3 4 5 6 7 1 chunk +2 lines, -4 lines 0 comments Download
M src/pkg/net/sockopt_posix.go View 1 2 3 4 5 1 chunk +7 lines, -14 lines 0 comments Download

Messages

Total messages: 25
dfc
Hello mikioh.mikioh@gmail.com, bradfitz@golang.org, dvyukov@google.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 7 months ago (2012-11-28 10:13:47 UTC) #1
dvyukov
https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/sockopt_posix.go File src/pkg/net/sockopt_posix.go (left): https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/sockopt_posix.go#oldcode122 src/pkg/net/sockopt_posix.go:122: func setReadDeadline(fd *netFD, t time.Time) error { This was ...
9 years, 7 months ago (2012-11-28 10:29:34 UTC) #2
dfc
On 2012/11/28 10:29:34, dvyukov wrote: > https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/sockopt_posix.go > File src/pkg/net/sockopt_posix.go (left): > > https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/sockopt_posix.go#oldcode122 > ...
9 years, 7 months ago (2012-11-28 11:06:39 UTC) #3
bradfitz
https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/fd_unix.go File src/pkg/net/fd_unix.go (right): https://codereview.appspot.com/6855110/diff/8001/src/pkg/net/fd_unix.go#newcode44 src/pkg/net/fd_unix.go:44: // read and write deadlines (nsec since 1970 or ...
9 years, 7 months ago (2012-11-28 14:21:03 UTC) #4
brainman
On 2012/11/28 11:06:39, dfc wrote: > ... > ... I did do a cross compile ...
9 years, 7 months ago (2012-11-29 04:17:19 UTC) #5
dfc
Thanks for your comments. I'm addressing them and will post another patch set this evening. ...
9 years, 7 months ago (2012-11-29 04:19:15 UTC) #6
brainman
On 2012/11/29 04:19:15, dfc wrote: > ... I don't know why my > crosscompile script ...
9 years, 7 months ago (2012-11-29 04:21:56 UTC) #7
dfc
Thank you for your comments and your testing. In order to resolve the changes to ...
9 years, 7 months ago (2012-11-29 06:31:36 UTC) #8
dfc
Hello mikioh.mikioh@gmail.com, bradfitz@golang.org, dvyukov@google.com, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-11-29 06:32:17 UTC) #9
bradfitz
https://codereview.appspot.com/6855110/diff/7012/src/pkg/net/fd_unix.go File src/pkg/net/fd_unix.go (right): https://codereview.appspot.com/6855110/diff/7012/src/pkg/net/fd_unix.go#newcode223 src/pkg/net/fd_unix.go:223: var deadline = s.deadline := https://codereview.appspot.com/6855110/diff/7012/src/pkg/net/fd_unix.go#newcode225 src/pkg/net/fd_unix.go:225: deadline -= ...
9 years, 7 months ago (2012-11-29 06:41:45 UTC) #10
bradfitz
LGTM None of those are real problems. I just don't like the name of that ...
9 years, 7 months ago (2012-11-29 06:44:08 UTC) #11
bradfitz
LGTM Nevermind, the Windows changes are identical to the POSIX ones, which is why I ...
9 years, 7 months ago (2012-11-29 06:44:51 UTC) #12
dfc
Thanks Brad. I need to wait for Alex and Mikio to confirm I haven't broken ...
9 years, 7 months ago (2012-11-29 06:50:06 UTC) #13
mikio
LGTM https://codereview.appspot.com/6855110/diff/7014/src/pkg/net/fd_posix_test.go File src/pkg/net/fd_posix_test.go (right): https://codereview.appspot.com/6855110/diff/7014/src/pkg/net/fd_posix_test.go#newcode14 src/pkg/net/fd_posix_test.go:14: var deadlineSetTimeTests = []struct { deadlineSetValueTests?
9 years, 7 months ago (2012-11-29 12:39:47 UTC) #14
mikio
> LGTM Hold on, sendfile_freebsd.go needs a twiddle too.
9 years, 7 months ago (2012-11-29 13:25:53 UTC) #15
dfc
Hello mikioh.mikioh@gmail.com, bradfitz@golang.org, dvyukov@google.com, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-11-29 21:58:05 UTC) #16
dfc
Thanks for your comments, I had missed hg add'ing sendfile_freebsd.go. PTAL. https://codereview.appspot.com/6855110/diff/7014/src/pkg/net/fd_posix_test.go File src/pkg/net/fd_posix_test.go (right): ...
9 years, 7 months ago (2012-11-29 21:58:30 UTC) #17
mikio
LGTM
9 years, 7 months ago (2012-11-29 22:37:05 UTC) #18
bradfitz
https://codereview.appspot.com/6855110/diff/12008/src/pkg/net/fd_unix.go File src/pkg/net/fd_unix.go (right): https://codereview.appspot.com/6855110/diff/12008/src/pkg/net/fd_unix.go#newcode225 src/pkg/net/fd_unix.go:225: // TODO(dfc) is this a time, or a duration ...
9 years, 7 months ago (2012-11-29 22:47:15 UTC) #19
bradfitz
https://codereview.appspot.com/6855110/diff/12008/src/pkg/net/fd_unix.go File src/pkg/net/fd_unix.go (right): https://codereview.appspot.com/6855110/diff/12008/src/pkg/net/fd_unix.go#newcode225 src/pkg/net/fd_unix.go:225: // TODO(dfc) is this a time, or a duration ...
9 years, 7 months ago (2012-11-29 22:48:18 UTC) #20
brainman
LGTM
9 years, 7 months ago (2012-11-29 23:09:39 UTC) #21
dfc
ok, all done. Going to smoke test this a bit more, then submit after work ...
9 years, 7 months ago (2012-11-29 23:40:36 UTC) #22
dfc
Hello mikioh.mikioh@gmail.com, bradfitz@golang.org, dvyukov@google.com, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 7 months ago (2012-11-29 23:41:01 UTC) #23
bradfitz
LGTM
9 years, 7 months ago (2012-11-29 23:41:35 UTC) #24
dfc
9 years, 7 months ago (2012-11-30 07:27:05 UTC) #25
*** Submitted as https://code.google.com/p/go/source/detail?r=869253ef7009 ***

net: fix data races on deadline vars

Fixes issue 4434.

R=mikioh.mikioh, bradfitz, dvyukov, alex.brainman
CC=golang-dev
https://codereview.appspot.com/6855110
Sign in to reply to this message.

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