Grover-c13 / PokeGOAPI-Java

Pokemon Go API
GNU General Public License v3.0
1.05k stars 334 forks source link

Current API unusable in multithreaded environment #275

Closed Paul776 closed 7 years ago

Paul776 commented 8 years ago

You have introduced this CredentialsProvider that is far to be thread safe. As a result I cannot not use ONE instance of it only... but will have to create n object for n thread And because CredentialsProvider handle the login itself it will log me in n times.. I don't want to have my accounts ban or locked because of this.

The best to do would be to make PokemonGO object thread safe, (by using some kind of thread pool) the end goal will be for a API user:

1) Create ONE single PokemonGO object by feeding the the creds 2) I can the safely pass PokemonGO to my task and not to worry about anything.

As a work around Im going to go back to the previous version where the login generate an AuthInfo (which be the way is Serializable) and I feed this to PokemonGO this way the only draw back will be to create N times a PokemonGO object but at least I don't log in N times

Aphoh commented 8 years ago

Anyone have experience with Java Concurrency?

vmarchaud commented 8 years ago

Dont get what you want to do exactly, you can already create a PokemonGo instance by invoking him with creds provider, after you pass it to your task and it can do whatever he want to with, our lib isnt using thread, you will get the response only in the thread where you asked for it. Can you explain more about the context of your concurrent problem ?

rajulbhatnagar commented 8 years ago

Ya currently the credential provider is not thread safe. Ill take a stab at this later today. The PokemonGo object should be thread safe. You can let me know where u think a Race in PokemonGo can occur(assuming the cred provider is thread safe).

Paul776 commented 8 years ago

@vmarchaud Neither PokemonGO nor CredentialsProvider are thread safe, meaning I cannot have ONE single instance for the all project (which would perfect and ideal)

This mean at the moment if I have 4 thread I need 4 PokemenGO objects and 4 CredentialsProvider objects This result in calling 4 times .login() and getPlayerProfile()...

But keep in mind.. that overtime I do a scan more than 250 thread (in a pool of 4) are started... Log-in 250 times is certainly not very good for the account...

I didn't have this problem in the previous version when .login() were to be called manually and not managed by the CredentialsProvider.

I do have profession experience in low level concurrency in java as per my job. unfortunately not very much time at the moment.

If none of you guys are able to look into this, let me know and I might try to have go to it.

Jari27 commented 8 years ago

Can you explain what the problem is exactly? I've been running 4 bots in 4 different threads using different PokemonGo objects and I've yet to see any problem.

bolds07 commented 8 years ago

There is no reason to make PokemonGO nor CredentialsProvider thread safe... unless you want to make clear to niantic server you are a bot...

Think clear, humans do each activity at once, if you are planning to use several threads to control diferents bahaviors so you are probably creating a non human capability... Well if you still want to go with this idea you just need to add the synchronized to every method that send server request

vmarchaud commented 8 years ago

@bolds07 He got his reason to want a thread-safe implementation, like android using multiple async task to make request etc. The API in is current state is making clear that you are using a bot like said in the readme.

@Paul776 Like i said, the only think that arent thread safe is the renew of access token, all methods that can be call within the api are executed within the thread of your call.

s7092910 commented 8 years ago

I'm not sure if people know this yet, but Niantic has started throttling API calls. So doing a call after call so quickly, even concurrently will have requests fail.

vmarchaud commented 8 years ago

We are currently trying the implement a async thread that will send request to try handle the rate-limit within the lib, it should btw be more concurrent.