Skip to content

Commit e5099d7

Browse files
dmitshurdmitshur
committed
Fix data race surrounding Client.Rate.
Previously, concurrent calls to Client.Do would have data races writing Client.Rate. Protect it with a mutex. Resolves #253.
1 parent 44b1ede commit e5099d7

2 files changed

Lines changed: 29 additions & 18 deletions

File tree

‎github/github.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"reflect"
1818
"strconv"
1919
"strings"
20+
"sync"
2021
"time"
2122

2223
"github.com/google/go-querystring/query"
@@ -64,11 +65,8 @@ type Client struct {
6465
// User agent used when communicating with the GitHub API.
6566
UserAgent string
6667

67-
// Rate specifies the current rate limit for the client as determined by the
68-
// most recent API call. If the client is used in a multi-user application,
69-
// this rate may not always be up-to-date. Call RateLimits() to check the
70-
// current rate.
71-
Rate Rate
68+
rateMu sync.Mutex
69+
rate Rate
7270

7371
// Services used for talking to different parts of the GitHub API.
7472
Activity *ActivityService
@@ -292,6 +290,17 @@ func (r *Response) populateRate() {
292290
}
293291
}
294292

293+
// Rate specifies the current rate limit for the client as determined by the
294+
// most recent API call. If the client is used in a multi-user application,
295+
// this rate may not always be up-to-date. Call RateLimits() to check the
296+
// current rate.
297+
func (c *Client) Rate() Rate {
298+
c.rateMu.Lock()
299+
rate := c.rate
300+
c.rateMu.Unlock()
301+
return rate
302+
}
303+
295304
// Do sends an API request and returns the API response. The API response is
296305
// JSON decoded and stored in the value pointed to by v, or returned as an
297306
// error if an API error has occurred. If v implements the io.Writer
@@ -307,7 +316,9 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
307316

308317
response := newResponse(resp)
309318

310-
c.Rate = response.Rate
319+
c.rateMu.Lock()
320+
c.rate = response.Rate
321+
c.rateMu.Unlock()
311322

312323
err = CheckResponse(resp)
313324
if err != nil {

‎github/github_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,28 +378,28 @@ func TestDo_rateLimit(t *testing.T) {
378378
w.Header().Add(headerRateReset, "1372700873")
379379
})
380380

381-
if got, want := client.Rate.Limit, 0; got != want {
381+
if got, want := client.Rate().Limit, 0; got != want {
382382
t.Errorf("Client rate limit = %v, want %v", got, want)
383383
}
384-
if got, want := client.Rate.Remaining, 0; got != want {
384+
if got, want := client.Rate().Remaining, 0; got != want {
385385
t.Errorf("Client rate remaining = %v, got %v", got, want)
386386
}
387-
if !client.Rate.Reset.IsZero() {
387+
if !client.Rate().Reset.IsZero() {
388388
t.Errorf("Client rate reset not initialized to zero value")
389389
}
390390

391391
req, _ := client.NewRequest("GET", "/", nil)
392392
client.Do(req, nil)
393393

394-
if got, want := client.Rate.Limit, 60; got != want {
394+
if got, want := client.Rate().Limit, 60; got != want {
395395
t.Errorf("Client rate limit = %v, want %v", got, want)
396396
}
397-
if got, want := client.Rate.Remaining, 59; got != want {
397+
if got, want := client.Rate().Remaining, 59; got != want {
398398
t.Errorf("Client rate remaining = %v, want %v", got, want)
399399
}
400400
reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC)
401-
if client.Rate.Reset.UTC() != reset {
402-
t.Errorf("Client rate reset = %v, want %v", client.Rate.Reset, reset)
401+
if client.Rate().Reset.UTC() != reset {
402+
t.Errorf("Client rate reset = %v, want %v", client.Rate().Reset, reset)
403403
}
404404
}
405405

@@ -418,15 +418,15 @@ func TestDo_rateLimit_errorResponse(t *testing.T) {
418418
req, _ := client.NewRequest("GET", "/", nil)
419419
client.Do(req, nil)
420420

421-
if got, want := client.Rate.Limit, 60; got != want {
421+
if got, want := client.Rate().Limit, 60; got != want {
422422
t.Errorf("Client rate limit = %v, want %v", got, want)
423423
}
424-
if got, want := client.Rate.Remaining, 59; got != want {
424+
if got, want := client.Rate().Remaining, 59; got != want {
425425
t.Errorf("Client rate remaining = %v, want %v", got, want)
426426
}
427427
reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC)
428-
if client.Rate.Reset.UTC() != reset {
429-
t.Errorf("Client rate reset = %v, want %v", client.Rate.Reset, reset)
428+
if client.Rate().Reset.UTC() != reset {
429+
t.Errorf("Client rate reset = %v, want %v", client.Rate().Reset, reset)
430430
}
431431
}
432432

@@ -453,7 +453,7 @@ func TestCheckResponse(t *testing.T) {
453453
res := &http.Response{
454454
Request: &http.Request{},
455455
StatusCode: http.StatusBadRequest,
456-
Body: ioutil.NopCloser(strings.NewReader(`{"message":"m",
456+
Body: ioutil.NopCloser(strings.NewReader(`{"message":"m",
457457
"errors": [{"resource": "r", "field": "f", "code": "c"}]}`)),
458458
}
459459
err := CheckResponse(res).(*ErrorResponse)

0 commit comments

Comments
 (0)