Skip to content

Commit

Permalink
httputil: Fixed unlock of unlocked mutex bug
Browse files Browse the repository at this point in the history
  • Loading branch information
diamondburned committed Aug 4, 2020
1 parent 2a032eb commit 94cca0a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
5 changes: 3 additions & 2 deletions api/rate/rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,15 @@ func (l *Limiter) Acquire(ctx context.Context, path string) error {
}

// Release releases the URL from the locks. This doesn't need a context for
// timing out, it doesn't block that much.
// timing out, since it doesn't block that much.
func (l *Limiter) Release(path string, headers http.Header) error {
b := l.getBucket(path, false)
if b == nil {
return nil
}

defer b.lock.Unlock()
// TryUnlock because Release may be called when Acquire has not been.
defer b.lock.TryUnlock()

// Check custom limiter
if b.custom != nil {
Expand Down
10 changes: 10 additions & 0 deletions internal/moreatomic/mutex.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ func (m *CtxMutex) Lock(ctx context.Context) error {
}
}

// TryUnlock returns true if the mutex has been unlocked.
func (m *CtxMutex) TryUnlock() bool {
select {
case <-m.mut:
return true
default:
return false
}
}

func (m *CtxMutex) Unlock() {
select {
case <-m.mut:
Expand Down
30 changes: 25 additions & 5 deletions utils/httputil/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,22 @@ func (c *Client) Context() context.Context {
return c.context
}

func (c *Client) applyOptions(r httpdriver.Request, extra []RequestOption) error {
// applyOptions tries to apply all options. It does not halt if a single option
// fails, and the error returned is the latest error.
func (c *Client) applyOptions(r httpdriver.Request, extra []RequestOption) (e error) {
for _, opt := range c.OnRequest {
if err := opt(r); err != nil {
return err
e = err
}
}

for _, opt := range extra {
if err := opt(r); err != nil {
return err
e = err
}
}

return nil
return
}

func (c *Client) MeanwhileMultipart(
Expand Down Expand Up @@ -158,25 +161,42 @@ func (c *Client) Request(method, url string, opts ...RequestOption) (httpdriver.
var r httpdriver.Response
var status int

// The c.Retries < 1 check ensures that we retry forever if that field is
// less than 1.
for i := uint(0); c.Retries < 1 || i < c.Retries; i++ {
q, err := c.Client.NewRequest(c.context, method, url)
if err != nil {
return nil, RequestError{err}
}

if err := c.applyOptions(q, opts); err != nil {
// We failed to apply an option, so we should call all OnResponse
// handler to clean everything up.
for _, fn := range c.OnResponse {
fn(q, nil)
}
// Exit after cleaning everything up.
return nil, errors.Wrap(err, "failed to apply options")
}

r, doErr = c.Client.Do(q)

// Error that represents the latest error in the chain.
var onRespErr error

// Call OnResponse() even if the request failed.
for _, fn := range c.OnResponse {
// Be sure to call ALL OnResponse handlers.
if err := fn(q, r); err != nil {
return nil, err
onRespErr = err
}
}

if onRespErr != nil {
return nil, errors.Wrap(err, "OnResponse handler failed")
}

// Retry if the request failed.
if doErr != nil {
continue
}
Expand Down

0 comments on commit 94cca0a

Please sign in to comment.