jnbt / candy_check

Check and verify in-app receipts
MIT License
125 stars 70 forks source link

Writing to PlayStore cache every boot! has concurrency issues #17

Closed ricky-crichq closed 4 years ago

ricky-crichq commented 6 years ago

When using this Gem we started seeing EOFError: end of file reached.

EOFError: end of file reached
[GEM_ROOT]/gems/activesupport-5.0.2/lib/active_support/core_ext/marshal.rb:4 :in `load`
[GEM_ROOT]/gems/activesupport-5.0.2/lib/active_support/core_ext/marshal.rb:4 :in `load`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/discovery_repository.rb:19 :in `block in load`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/discovery_repository.rb:18 :in `open`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/discovery_repository.rb:18 :in `load`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/client.rb:118 :in `load_discover_dump`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/client.rb:90 :in `discover!`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/client.rb:40 :in `boot!`
[GEM_ROOT]/gems/candy_check-0.1.1/lib/candy_check/play_store/verifier.rb:24 :in `boot!`

We are pretty sure this is caused by multiple threads trying to save/load the PlayStore Discovery Repository at the same time.

This can be replicated pretty easily:

#!/usr/local/bin/ruby -w
require 'candy_check'

CACHE_FILE = '/tmp/candy_check_play_store_cache'

api_client = Google::APIClient.new(
  application_name: 'test',
  application_version: '0.0.1'
)
rpc = api_client.discovered_api('androidpublisher', 'v2')

def load_discover_dump
  CandyCheck::PlayStore::DiscoveryRepository.new(CACHE_FILE).load
end

def write_discover_dump(rpc)
  CandyCheck::PlayStore::DiscoveryRepository.new(CACHE_FILE).save(rpc)
end

t1 = Thread.new do
  write_discover_dump(rpc)
end

t2 = Thread.new do
  load_discover_dump
end

[t1, t2].map(&:join)

Not really sure how best to proceed here but one way to improve this would be to not rewrite the cached file on every boot. Unless there is something I am missing here?

https://github.com/jnbt/candy_check/blob/master/lib/candy_check/play_store/client.rb#L92

Happy to put in the work to improve this with some guidance on the best way to proceed.

klaseskilson commented 6 years ago

Good find! Would love some help and/or a PR if you have the time. I guess one thing could be, as you say, to avoid creating the file on every boot.

ricky-crichq commented 6 years ago

Yeap, no problem, we will make some changes to not rewrite the file if it just read from the cache.

jnbt commented 5 years ago

Will be also solved by #35

jnbt commented 4 years ago

Solved by #35 as the whole cache is not needed anymore. Please see the Migration Guide for more information.