Skip to content

Commit

Permalink
Fix suboptimal lock release in (*handler).getChainInfo. (#733)
Browse files Browse the repository at this point in the history
Also remove direct access to h.chainInfo.

No race condition currently possible because `h.chainInfo` is never set
to `nil` but, if it were, `getChainInfo` could sometimes unnecessarily return `nil`.

Possible source of bugs in the future.
  • Loading branch information
gnattishness committed Aug 4, 2020
1 parent 32e9b7e commit 84eedb1
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ func withCommonHeaders(version string, h func(http.ResponseWriter, *http.Request
}

type handler struct {
timeout time.Duration
client client.Client
timeout time.Duration
client client.Client
// NOTE: should only be accessed via getChainInfo
chainInfo *chain.Info
chainInfoLk sync.RWMutex
log log.Logger
Expand Down Expand Up @@ -149,8 +150,9 @@ RESET:
func (h *handler) getChainInfo(ctx context.Context) *chain.Info {
h.chainInfoLk.RLock()
if h.chainInfo != nil {
info := h.chainInfo
h.chainInfoLk.RUnlock()
return h.chainInfo
return info
}
h.chainInfoLk.RUnlock()

Expand All @@ -175,7 +177,7 @@ func (h *handler) getChainInfo(ctx context.Context) *chain.Info {
return info
}

func (h *handler) getRand(ctx context.Context, round uint64) ([]byte, error) {
func (h *handler) getRand(ctx context.Context, info *chain.Info, round uint64) ([]byte, error) {
h.startOnce.Do(h.start)
// First see if we should get on the synchronized 'wait for next release' bandwagon.
block := false
Expand Down Expand Up @@ -212,7 +214,7 @@ func (h *handler) getRand(ctx context.Context, round uint64) ([]byte, error) {
}

// make sure we aren't going to ask for a round that doesn't exist yet.
if time.Unix(chain.TimeOfRound(h.chainInfo.Period, h.chainInfo.GenesisTime, round), 0).After(time.Now()) {
if time.Unix(chain.TimeOfRound(info.Period, info.GenesisTime, round), 0).After(time.Now()) {
return nil, nil
}

Expand Down Expand Up @@ -253,7 +255,7 @@ func (h *handler) PublicRand(w http.ResponseWriter, r *http.Request) {
return
}

data, err := h.getRand(r.Context(), roundN)
data, err := h.getRand(r.Context(), info, roundN)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
h.log.Warn("http_server", "failed to get randomness", "client", r.RemoteAddr, "req", url.PathEscape(r.URL.Path), "err", err)
Expand Down

0 comments on commit 84eedb1

Please sign in to comment.