search

twilio / twilio-go

A Go package for communicating with the Twilio API.
MIT License
272 stars 40 forks source link

Data race after upgrade #157

Closed wavded closed 2 years ago

wavded commented 2 years ago

Issue Summary

After upgrading from v0.21.0 to v0.24.0, I have a data race when running the below function with go run -race

Steps to Reproduce

  1. Run go run -race with a call to client.ApiV2010.ListAvailablePhoneNumberTollFree

Code Snippet

client := twilio.NewRestClientWithParams(twilio.ClientParams{
    Username: id,
    Password: token,
})
limit := 1
numbers, err := client.ApiV2010.ListAvailablePhoneNumberTollFree("US",
    &openapi.ListAvailablePhoneNumberTollFreeParams{
        PageSize: &limit,
    })

Exception/Log

WARNING: DATA RACE
Write at 0x00c0004c43e0 by goroutine 47:
  github.com/twilio/twilio-go/rest/api/v2010.(*ApiService).StreamAvailablePhoneNumberTollFree.func1()
      /var/lib/jenkins/go/pkg/mod/github.com/twilio/twilio-go@v0.24.0/rest/api/v2010/accounts_available_phone_numbers_toll_free.go:293 +0x16d

Previous read at 0x00c0004c43e0 by goroutine 46:
  github.com/twilio/twilio-go/rest/api/v2010.(*ApiService).StreamAvailablePhoneNumberTollFree()
      /var/lib/jenkins/go/pkg/mod/github.com/twilio/twilio-go@v0.24.0/rest/api/v2010/accounts_available_phone_numbers_toll_free.go:303 +0x574
  github.com/twilio/twilio-go/rest/api/v2010.(*ApiService).ListAvailablePhoneNumberTollFree()
      /var/lib/jenkins/go/pkg/mod/github.com/twilio/twilio-go@v0.24.0/rest/api/v2010/accounts_available_phone_numbers_toll_free.go:250 +0x6e
  github.com/applieddataconsultants/dqm-api/twilio.(*Texter).AvailableNumber()
      /var/lib/jenkins/jobs/GH/jobs/dqm-api/branches/PR-147/workspace/twilio/texter.go:78 +0x144
  github.com/applieddataconsultants/dqm-api/twilio.TestTexter.func3()
      /var/lib/jenkins/jobs/GH/jobs/dqm-api/branches/PR-147/workspace/twilio/texter_test.go:44 +0x190
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47

Goroutine 47 (running) created at:
  github.com/twilio/twilio-go/rest/api/v2010.(*ApiService).StreamAvailablePhoneNumberTollFree()
      /var/lib/jenkins/go/pkg/mod/github.com/twilio/twilio-go@v0.24.0/rest/api/v2010/accounts_available_phone_numbers_toll_free.go:280 +0x567
  github.com/twilio/twilio-go/rest/api/v2010.(*ApiService).ListAvailablePhoneNumberTollFree()
      /var/lib/jenkins/go/pkg/mod/github.com/twilio/twilio-go@v0.24.0/rest/api/v2010/accounts_available_phone_numbers_toll_free.go:250 +0x6e
  github.com/applieddataconsultants/dqm-api/twilio.(*Texter).AvailableNumber()
      /var/lib/jenkins/jobs/GH/jobs/dqm-api/branches/PR-147/workspace/twilio/texter.go:78 +0x144
  github.com/applieddataconsultants/dqm-api/twilio.TestTexter.func3()
      /var/lib/jenkins/jobs/GH/jobs/dqm-api/branches/PR-147/workspace/twilio/texter_test.go:44 +0x190
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47

Goroutine 46 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1306 +0x726
  github.com/applieddataconsultants/dqm-api/twilio.TestTexter()
      /var/lib/jenkins/jobs/GH/jobs/dqm-api/branches/PR-147/workspace/twilio/texter_test.go:42 +0x7c5
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
==================
--- FAIL: TestTexter (2.38s)
    --- FAIL: TestTexter/AvailableNumber (0.53s)
        testing.go:1152: race detected during execution of test
    testing.go:1152: race detected during execution of test
FAIL

Technical details:

Seems to have been introduced here -> https://github.com/twilio/twilio-go/compare/v0.22.1...v0.24.0#diff-de50900a25591183bfd66763d56b5c812f8a2240004a592c9205ae33bb1d3126R293

childish-sambino commented 2 years ago

Ah, looks like there's always been a data race issue with the Stream... functions and we recently migrated the List... functions to call the Stream... functions internally to reduce duplicate code.

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.