Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: remove race around bodyCloser when no content #100

Merged
merged 1 commit into from Aug 12, 2019

Conversation

@ericrpowers
Copy link
Contributor

commented Aug 12, 2019

Description

This PR will aim to fix the following data race:

==================
WARNING: DATA RACE
Read at 0x00c0027b1710 by goroutine 227:
  bytes.(*Buffer).Read()
      /usr/local/go/src/bytes/buffer.go:72 +0x60
  io/ioutil.(*nopCloser).Read()
      <autogenerated>:1 +0x87
  net/http.transferBodyReader.Read()
      /usr/local/go/src/net/http/transfer.go:62 +0x77
  io.(*LimitedReader).Read()
      /usr/local/go/src/io/io.go:448 +0xc7
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:722 +0x13b
  io.copyBuffer()
      /usr/local/go/src/io/io.go:388 +0x3fa
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/io/io.go:364 +0xc3f
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:655 +0x85e
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:1979 +0x321

Previous write at 0x00c0027b1710 by goroutine 63:
  bytes.(*Buffer).Read()
      /usr/local/go/src/bytes/buffer.go:101 +0xb5
  io/ioutil.(*nopCloser).Read()
      <autogenerated>:1 +0x87
  io/ioutil.devNull.ReadFrom()
      /usr/local/go/src/io/ioutil/ioutil.go:147 +0xb9
  io/ioutil.(*devNull).ReadFrom()
      <autogenerated>:1 +0x7c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:388 +0x3fa
  go.aporeto.io/backend/vendor/go.aporeto.io/manipulate/maniphttp.(*httpManipulator).send.func2()
      /usr/local/go/src/io/io.go:364 +0xf9
  go.aporeto.io/backend/vendor/go.aporeto.io/manipulate/maniphttp.(*httpManipulator).send()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/vendor/go.aporeto.io/manipulate/maniphttp/manipulator.go:743 +0x1422
  go.aporeto.io/backend/vendor/go.aporeto.io/manipulate/maniphttp.(*httpManipulator).Create()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/vendor/go.aporeto.io/manipulate/maniphttp/manipulator.go:263 +0x633
  go.aporeto.io/backend/internal/tagutils.(*Batcher).handleTags.func1()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/internal/tagutils/batcher.go:108 +0x41f
  go.aporeto.io/backend/internal/tagutils.(*Batcher).handleTags()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/internal/tagutils/batcher.go:147 +0x449

Goroutine 227 (running) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1358 +0xc57
  net/http.(*Transport).getConn.func4()
      /usr/local/go/src/net/http/transport.go:1015 +0xd0

Goroutine 63 (running) created at:
  go.aporeto.io/backend/internal/tagutils.(*Batcher).Start()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/internal/tagutils/batcher.go:52 +0x68
  go.aporeto.io/backend/internal/bootstrap.MakeTagsInjector()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/internal/bootstrap/crud.go:25 +0x114
  go.aporeto.io/backend/srv/squall.Start()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/srv/squall/main.go:72 +0xcc3
  main.start()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/services.go:85 +0x558
  main.main()
      /tmp/build/ae5c2d4a/go/src/go.aporeto.io/backend/main.go:18 +0x4d
==================
@codecov

This comment has been minimized.

Copy link

commented Aug 12, 2019

Codecov Report

Merging #100 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   76.76%   76.78%   +0.02%     
==========================================
  Files          35       35              
  Lines        2569     2572       +3     
==========================================
+ Hits         1972     1975       +3     
  Misses        519      519              
  Partials       78       78
Impacted Files Coverage Δ
maniphttp/manipulator.go 96.65% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 937e853...e977f53. Read the comment docs.

@ericrpowers ericrpowers merged commit 038f96a into master Aug 12, 2019

4 checks passed

built
Details
codecov/patch 100% of diff hit (target 76.76%)
Details
codecov/project 76.78% (+0.02%) compared to 937e853
Details
unit-tests
Details

@ericrpowers ericrpowers deleted the race-fix branch Aug 12, 2019

ericrpowers added a commit that referenced this pull request Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.