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

Jump to bottom

pool: use copied raw db values outside of db tx. #123

Merged
merged 2 commits into from Aug 13, 2019
Merged

pool: use copied raw db values outside of db tx. #123

merged 2 commits into from Aug 13, 2019

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Aug 9, 2019 •
edited

This fixes data access issues related to using raw db values outside of their associated db txs and also simplifies the rate limiter.

Fixes #92.

jrick
jrick reviewed Aug 9, 2019
View reviewed changes
pool/hub.go Outdated Show resolved Hide resolved
@dnldd
pool: use copied raw db values outside of db tx.
07f9138 
This fixes data access issues related to using raw db
values outside of their associated db txs and also
simplifies the rate limiter.
@jholdstock
Copy link
Member

Discovered a race whilst testing this:

==================
WARNING: DATA RACE
Write at 0x00c0000ba940 by goroutine 30:
  runtime.slicecopy()
      /usr/lib/go-1.12/src/runtime/slice.go:197 +0x0
  math/big.(*Rat).Float64()
      /usr/lib/go-1.12/src/math/big/nat.go:94 +0x1e3
  github.com/decred/dcrpool/gui.ratToPercent()
      /code/decred/dcrpool/gui/formatting.go:94 +0x3c
  github.com/decred/dcrpool/gui.(*GUI).updateWS()
      /code/decred/dcrpool/gui/websocket.go:69 +0x782
  github.com/decred/dcrpool/gui.(*GUI).Run.func2()
      /code/decred/dcrpool/gui/gui.go:323 +0x1ae

Previous write at 0x00c0000ba940 by goroutine 64:
  runtime.slicecopy()
      /usr/lib/go-1.12/src/runtime/slice.go:197 +0x0
  math/big.(*Rat).Float64()
      /usr/lib/go-1.12/src/math/big/nat.go:94 +0x1e3
  github.com/decred/dcrpool/gui.ratToPercent()
      /code/decred/dcrpool/gui/formatting.go:94 +0x3c
  runtime.call32()
      /usr/lib/go-1.12/src/runtime/asm_amd64.s:519 +0x3a
  reflect.Value.Call()
      /usr/lib/go-1.12/src/reflect/value.go:308 +0xc0
  text/template.safeCall()
      /usr/lib/go-1.12/src/text/template/funcs.go:293 +0xdd
  text/template.(*state).evalCall()
      /usr/lib/go-1.12/src/text/template/exec.go:703 +0x6e0
  text/template.(*state).evalFunction()
      /usr/lib/go-1.12/src/text/template/exec.go:564 +0x1d7
  text/template.(*state).evalCommand()
      /usr/lib/go-1.12/src/text/template/exec.go:461 +0xb0f
  text/template.(*state).evalPipeline()
      /usr/lib/go-1.12/src/text/template/exec.go:430 +0x1ff
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:254 +0x69c
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:262 +0x1a0
  text/template.(*state).walkRange.func1()
      /usr/lib/go-1.12/src/text/template/exec.go:349 +0x14d
  text/template.(*state).walkRange()
      /usr/lib/go-1.12/src/text/template/exec.go:358 +0x6e1
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:265 +0x61a
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:262 +0x1a0
  text/template.(*state).walkIfOrWith()
      /usr/lib/go-1.12/src/text/template/exec.go:292 +0x1eb
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:259 +0x49c
  text/template.(*state).walk()
      /usr/lib/go-1.12/src/text/template/exec.go:262 +0x1a0
  text/template.(*Template).execute()
      /usr/lib/go-1.12/src/text/template/exec.go:217 +0x2f8
  html/template.(*Template).ExecuteTemplate()
      /usr/lib/go-1.12/src/text/template/exec.go:200 +0xf5
  github.com/decred/dcrpool/gui.(*GUI).renderTemplate()
      /code/decred/dcrpool/gui/gui.go:113 +0xfa
  github.com/decred/dcrpool/gui.(*GUI).GetIndex()
      /code/decred/dcrpool/gui/index.go:94 +0x16cd
  github.com/decred/dcrpool/gui.(*GUI).GetIndex-fm()
      /code/decred/dcrpool/gui/index.go:46 +0x5f
  net/http.HandlerFunc.ServeHTTP()
      /usr/lib/go-1.12/src/net/http/server.go:1995 +0x51
  github.com/gorilla/csrf.(*csrf).ServeHTTP()
      /code/gopath/pkg/mod/github.com/gorilla/csrf@v1.5.1/csrf.go:267 +0x71c
  github.com/gorilla/mux.(*Router).ServeHTTP()
      /code/gopath/pkg/mod/github.com/gorilla/mux@v1.7.2/mux.go:212 +0x13f
  net/http.serverHandler.ServeHTTP()
      /usr/lib/go-1.12/src/net/http/server.go:2774 +0xce
  net/http.initNPNRequest.ServeHTTP()
      /usr/lib/go-1.12/src/net/http/server.go:3323 +0xfc
  net/http.(*initNPNRequest).ServeHTTP()
      <autogenerated>:1 +0x94
  net/http.Handler.ServeHTTP-fm()
      /usr/lib/go-1.12/src/net/http/server.go:86 +0x64
  net/http.(*http2serverConn).runHandler()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:5658 +0x96

Goroutine 30 (running) created at:
  github.com/decred/dcrpool/gui.(*GUI).Run()
      /code/decred/dcrpool/gui/gui.go:283 +0x86
  main.main()
      /code/decred/dcrpool/dcrpool.go:193 +0x641

Goroutine 64 (finished) created at:
  net/http.(*http2serverConn).processHeaders()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:5392 +0x917
  net/http.(*http2serverConn).processFrame()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:4923 +0x59d
  net/http.(*http2serverConn).processFrameFromReader()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:4881 +0x7af
  net/http.(*http2serverConn).serve()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:4397 +0x1252
  net/http.(*http2Server).ServeConn()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:3996 +0xd7b
  net/http.http2ConfigureServer.func1()
      /usr/lib/go-1.12/src/net/http/h2_bundle.go:3834 +0xb8
  net/http.(*conn).serve()
      /usr/lib/go-1.12/src/net/http/server.go:1802 +0x1b33
==================

@dnldd
Copy link
Member Author

dnldd commented Aug 10, 2019

Just noticed the data set is being accessed in the template as well:

<tbody>
    {{ range .WorkQuotas }}
    <tr data-row-id="{{ truncateAccountID .AccountID}}">
        <td>{{ truncateAccountID .AccountID}}</td>
        <td>{{ratToPercent .Percentage}}</td>
    </tr>
    {{end}}
</tbody>

We'll have to do all data formatting outside the templates and just read instead. On it.

jholdstock
jholdstock approved these changes Aug 12, 2019
View reviewed changes
Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me. Code changes look OK and I'm not seeing any issues with -race or -msan. Also I can no longer re-create the error from #92

@degeri
Copy link
Member

degeri commented Aug 13, 2019 •
edited

Checked, not seeing any race issues when using the website.
But Found another race on the miner ports.

curl http://127.0.0.1:5550/ produces .

WARNING: DATA RACE
Write at 0x00c0000d4cd0 by goroutine 54:
  runtime.closechan()
      /usr/local/go/src/runtime/chan.go:334 +0x0
  github.com/decred/dcrpool/pool.(*Client).shutdown()
      /home/root/workdir/dcrpool/pool/client.go:155 +0xca
  github.com/decred/dcrpool/pool.(*Client).run()
      /home/root/workdir/dcrpool/pool/client.go:859 +0x16b

Previous read at 0x00c0000d4cd0 by goroutine 55:
  runtime.chansend()
      /usr/local/go/src/runtime/chan.go:142 +0x0
  github.com/decred/dcrpool/pool.(*Client).read()
      /home/root/workdir/dcrpool/pool/client.go:571 +0x10a

Goroutine 54 (running) created at:
  github.com/decred/dcrpool/pool.(*endpoint).connect()
      /home/root/workdir/dcrpool/pool/endpoint.go:106 +0xdc

Goroutine 55 (running) created at:
  github.com/decred/dcrpool/pool.(*Client).run()
      /home/root/workdir/dcrpool/pool/client.go:851 +0x8a

2019-08-13 xx:27:30.808 [TRC] POOL: Client (86b67fa5/innosilicond9) removed.
2019-08-13 xx:27:30.808 [TRC] POOL: Connection to (86b67fa5/innosilicond9) terminated.
panic: send on closed channel

goroutine 23 [running]:
github.com/decred/dcrpool/pool.(*Client).read(0xc00049c000)
        /home/root/workdir/dcrpool/pool/client.go:571 +0x10b
created by github.com/decred/dcrpool/pool.(*Client).run
        /home/root/workdir/dcrpool/pool/client.go:851 +0x8b

@degeri
Copy link
Member

degeri commented Aug 13, 2019

ce33e8e fixes the issue above

@dnldd dnldd merged commit 3237fda into decred:master Aug 13, 2019
@jholdstock jholdstock mentioned this pull request Jun 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrick jrick jrick left review comments

@jholdstock jholdstock jholdstock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

crash: unexpected fault address
4 participants
@dnldd @jholdstock @degeri @jrick