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

Crash on concurrent access of resources #10

Closed
mjadczak opened this issue Aug 14, 2017 · 3 comments · Fixed by #12
Closed

Crash on concurrent access of resources #10

mjadczak opened this issue Aug 14, 2017 · 3 comments · Fixed by #12
Assignees

Comments

@mjadczak
Copy link

mjadczak commented Aug 14, 2017

I'm using stripe-mock to run a test suite for a Stripe client library for Elixir. I only just realised that stripe-mock responds based on fixtures and does not keep internal state, and so I switched the test suite over to be asynchronous, such that many tests can execute at a time.

However, it seems that stripe-mock cannot handle concurrent access right now. Here is the output with -verbose enabled. It seems to me that the internal fixture map is being accessed at the same time in an unsafe way. I'm not much of a Go programmer so I don't think I can help out much more than that.

This is probably low priority since a simple fix is to just ensure that access is serial, but allowing concurrent access would be good for running large test suites quickly.

@brandur-stripe
Copy link
Contributor

Oh no! Good concurrency is the whole point of using something like Go.

Thanks for filing. It looks like the fix should be pretty straightforward. I'll look into it.

@brandur-stripe brandur-stripe self-assigned this Aug 14, 2017
@brandur-stripe
Copy link
Contributor

Alright, I've got a good handle on this problem, and am quite embarrassed at how amateur it was :) I'm going to try and write a few regression tests, but I should have a fix out by tomorrow.

brandur-stripe pushed a commit that referenced this issue Aug 15, 2017
Previously running concurrency operations against the generator would
result in a panic because it was actually taking maps that it found and
fixtures and mutating them, resulting in concurrent reads and writes.

This patch introduces a fix that copies fixtures that are found in the
decoded set so that each Goroutine is working with a local copy. It also
introduces testing to protect against regressions.

Fixes #10.
@brandur-stripe
Copy link
Contributor

Shipped a fixed version as 0.1.16.

mjadczak added a commit to beam-community/stripity-stripe that referenced this issue Oct 15, 2017
mjadczak added a commit to beam-community/stripity-stripe that referenced this issue Oct 17, 2017
…nguages (#252)

* Add initial StripeMock code, which uses the [stripe_mock](https://github.com/stripe/stripe-mock) tool to make it available for tests

* Reimplement StripeMock using :exexec to make it more reliable

* Make various tweaks to harness
Test Stripe.Account

* Tweak Stripe.Account test

* Add tests for Stripe.Charge

* Update Travis dist (try to fix g++ out of date bug)

* Add stripe-mock grabbing to the Travis build

* Add stripe-mock grabbing to the Travis build

* Add tests for Stripe.ApplePayDomain

* Fix typo

* Move Stripe object tests into their own directory

* Add tests for Stripe.Coupon

* Add tests for Stripe.Customer
Tweak stripe-mock harness

* Add tests for Stripe.Invoice

* Add tests for Stripe.Plan

* Add tests for Stripe.Refund

* Add tests for Stripe.Subscription

* Move object modules into their own directory

* Recognise more Stripe objects

* Merge and finish @asummers' work asummers:remove-null-checks on removing Stripe.Changeset

* Disable "is saveable" tests until #248 resolves whether things actually should be saveable

* Allow deletion by struct and id (fix #250)
Change tests to expect `{:ok, struct}` from `delete`

* Allow invoices to be paid (see #126)

* Add :discount to Stripe.Subscription
Add Stripe.Subscription.delete_discount (see #126)

* Add check for missing fields in structs

* Remove Stripe resets, make tests async

* Make tests sync again until stripe/stripe-mock#10 is fixed

* Fixes terminology, removes redundant tests and allows async testing
"is saveable" tests removed (see #248)
stripe-mock bumped to 1.1.17, allowing async testing
terminology referring to functions corrected for style

* Fix missed function reference

* Move files in accordance with Stripe docs organisation

* Major commit with various organizational and internal changes

- Adds `Stripe.Error`, a unified struct for errors
- Moves the internal hackney request code into `Stripe.API`
- Turns `Stripe.Request` into a composable request struct
  providing a new internal API for requests
- Adds `Stripe.Entity`, a behaviour which allows object structs
  to modify the structure of the returned JSON
- Adds `Stripe.Balance` and `Stripe.BalanceTransaction`
- Adds tests for `Stripe.BalanceTransaction` as the first module
  to use the `Stripe.Request` API.

* Fix `defoverridable` call which broke compilation on Elixir 1.3

* Bring `Stripe.Balance` up to date with new API
Style tweak to `Stripe.BalanceTransaction`

* Bump API version to 2017-08-15

* Stop validating required or valid fields on client-side

* Add cast_path/3 to Stripe.Entity

* Guard Stripe.Entity casting functions against nils or missing keys

* Do not turn metadata keys to atoms

* Implement Stripe.Charge

* Implement Stripe.Refund

* Add log_deprecation functionality

* Log deprecations for old Charge functions

* Tag non-core Charge tests appropriately

* Mark Card as a Stripe.Entity

* Document Stripe.Entity

* Test Stripe.Entity

* Document Stripe.Request

* Tweak Travis matrix to test major Elixir and OTP versions, bump mix version to 2.0.0-beta.1

* Add initial StripeMock code, which uses the [stripe_mock](https://github.com/stripe/stripe-mock) tool to make it available for tests

* Reimplement StripeMock using :exexec to make it more reliable

* Make various tweaks to harness
Test Stripe.Account

* Tweak Stripe.Account test

* Add tests for Stripe.Charge

* Update Travis dist (try to fix g++ out of date bug)

* Add stripe-mock grabbing to the Travis build

* Add stripe-mock grabbing to the Travis build

* Add tests for Stripe.ApplePayDomain

* Fix typo

* Move Stripe object tests into their own directory

* Add tests for Stripe.Coupon

* Add tests for Stripe.Customer
Tweak stripe-mock harness

* Add tests for Stripe.Invoice

* Add tests for Stripe.Plan

* Add tests for Stripe.Refund

* Add tests for Stripe.Subscription

* Move object modules into their own directory

* Recognise more Stripe objects

* Merge and finish @asummers' work asummers:remove-null-checks on removing Stripe.Changeset

* Disable "is saveable" tests until #248 resolves whether things actually should be saveable

* Allow deletion by struct and id (fix #250)
Change tests to expect `{:ok, struct}` from `delete`

* Allow invoices to be paid (see #126)

* Add :discount to Stripe.Subscription
Add Stripe.Subscription.delete_discount (see #126)

* Add check for missing fields in structs

* Remove Stripe resets, make tests async

* Make tests sync again until stripe/stripe-mock#10 is fixed

* Fixes terminology, removes redundant tests and allows async testing
"is saveable" tests removed (see #248)
stripe-mock bumped to 1.1.17, allowing async testing
terminology referring to functions corrected for style

* Fix missed function reference

* Move files in accordance with Stripe docs organisation

* Major commit with various organizational and internal changes

- Adds `Stripe.Error`, a unified struct for errors
- Moves the internal hackney request code into `Stripe.API`
- Turns `Stripe.Request` into a composable request struct
  providing a new internal API for requests
- Adds `Stripe.Entity`, a behaviour which allows object structs
  to modify the structure of the returned JSON
- Adds `Stripe.Balance` and `Stripe.BalanceTransaction`
- Adds tests for `Stripe.BalanceTransaction` as the first module
  to use the `Stripe.Request` API.

* Fix `defoverridable` call which broke compilation on Elixir 1.3

* Bring `Stripe.Balance` up to date with new API
Style tweak to `Stripe.BalanceTransaction`

* Bump API version to 2017-08-15

* Stop validating required or valid fields on client-side

* Add cast_path/3 to Stripe.Entity

* Guard Stripe.Entity casting functions against nils or missing keys

* Do not turn metadata keys to atoms

* Implement Stripe.Charge

* Implement Stripe.Refund

* Add log_deprecation functionality

* Log deprecations for old Charge functions

* Tag non-core Charge tests appropriately

* Mark Card as a Stripe.Entity

* Document Stripe.Entity

* Test Stripe.Entity

* Document Stripe.Request

* Tweak Travis matrix to test major Elixir and OTP versions, bump mix version to 2.0.0-beta.1

* Exclude invalid Elixir / OTP version combination from Travis build

* Fix travis config exclusion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants