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

Fix race condition with multiple requests (android) #942

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

moperacz
Copy link

@moperacz moperacz commented Mar 5, 2015

Explanation of this issue
#873

not sure what exactly the lines 714-716 are doing... so i deleted them 😀 (works for me)

@aogilvie
Copy link
Collaborator

aogilvie commented Mar 6, 2015

@moperacz Hi thanks for the PR. Can you provide test code? Thanks.

@moperacz
Copy link
Author

moperacz commented Mar 6, 2015

test as in java tests? or javascript "proof" that this works 😉

the javascript is the same as in #873

(copied from @ggmichaelgo issue)

facebookConnectPlugin.api('/me', null, function(response){
  console.log('me');
  console.log(response);
});
setTimeout(function(){
  facebookConnectPlugin.api('/me/permissions', null, function(response){
    console.log('permission');
    console.log(response);
  });
}, 70)

In the console, only the "/me/permissions" request will get returned with the "/me" request's response data.
This is happening because on the 2nd call, the FacebookConnectPlugin.m sets the graphCallbackId value to 2nd call's callbackId.
So, the 1st call will get the response from the Facebook API first, and then callback to 2nd request.

thx

@moperacz
Copy link
Author

any news about this pr?

@aogilvie
Copy link
Collaborator

I'll taste a look this week. I'm behind on tracking fb plugin related issues. However, anyone is open to review it!

@mwillbanks
Copy link

Have experienced this same issue; merged patch into my own branch and works like a charm. The code change is quite simple and relatively self contained. @aogilvie can you please merge?

@mwillbanks
Copy link

This race condition also applies on the iOS side.

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 this pull request may close these issues.

3 participants