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

Proper way to wait until prepare is done #152

Closed
axelkennedal opened this issue May 5, 2018 · 18 comments
Closed

Proper way to wait until prepare is done #152

axelkennedal opened this issue May 5, 2018 · 18 comments
Labels
🤖 android Related to android 🙏 help wanted Extra attention is needed

Comments

@axelkennedal
Copy link

axelkennedal commented May 5, 2018

Version of react-native-iap

0.3.23

Platforms you faced the problem (IOS or Android or both?)

Android

Problem

Some methods of this library are getting called before prepare() is done.

Description

I'm wondering what the recommended way is for waiting until the library is ready to process requests on Android. The example provided in the README cannot really be applied directly to my application, since what I'm doing is I've created a class with static functions for handling In App Purchases (kind of an abstraction on top of this library). In my App.js I call prepare() in componentDidMount(), and so the issue is that the other component of my app will load and call getProducts(itemSKUs) before componentDidMount() (and therewith prepare()) is done executing.

My current (not very pleasing) solution is to just set a timeout of 1 second in the other component before calling getProducts(itemSKUs) from it.

I had an idea about setting a boolean like this and then making it so that the other functions have to wait for it to be true before they can execute, but I'm not sure how to accomplish that.

export default class IAPManager {
    static hasUnlockedPremium = undefined;
    static productInfo = undefined;
    static isReady = undefined;

    static async prepare() {
        const isConnected = await NetInfo.isConnected.fetch();
        if (isConnected) {
            try {
                await InAppPurchase.prepare();
                IAPManager.isReady = true;
            } catch (e) {
                console.log("Could not prepare IAP", e);
                IAPManager.isReady = false;
            }
        } else {
            IAPManager.isReady = false;
        }
    }
...
}
@hyochan hyochan added the 🙏 help wanted Extra attention is needed label May 6, 2018
@hyochan
Copy link
Owner

hyochan commented May 6, 2018

What happens if you try to prepare with await before calling getProducts in componentDidMount?

@axelkennedal
Copy link
Author

The point is that I want my class to be a manager of all things related to IAP in my app. and it should be initialized when the app launches. I want to use it in another component aswell, so I don't think putting the prepare() in the componentDidMount() in my other component (not App.js) would be the best idea.

Can I call prepare multiple times without any issues? If I use functions from this library in multiple places, should I just prepare() everywhere in my code before I use the library functions? Seems like bad design, I'd like to just prepare once and then be able to use it once it's ready.

@axelkennedal
Copy link
Author

Perhaps a mutex could be used to solve this issue? I'm thinking that the mutex would be locked in the beginning of the prepare() of my class, and that the other functions of my class would wait until that mutex is unlocked. I found this library for JS mutexes.

@hyochan
Copy link
Owner

hyochan commented May 6, 2018

@axelkennedal You can use prepare method anywhere, but be sure that you call endConnection when you are not needing it. Therefore, call prepare in componentDidMount and call endConnection in componentWillUnmount will be fine. If you call prepare without calling endConnection, there may be some bug because it is opening multiple threads that are not doing anything or may corrupt with the previous ones.

@axelkennedal
Copy link
Author

But what if I use the library in two components that load at the same time? Isn't it highly likely then that prepare() will be called simultaneously in both components and this creating a race condition?

@hyochan
Copy link
Owner

hyochan commented May 7, 2018

@axelkennedal You need your programming skill to solve this kind of problem currently. If I were you, I would set one global variable that manages if you've called prepare method or not. I'll write some code that might help you to solve this kind of problem. You may find better solution tough.

let openedBilling = false;  // this variable will stay globally somewhere like `global.js`

// component 1
async componentDidMount() {
  if (!openBilling) {
    await RNIap.prepare();
  }
  const products = await RNIap.getProducts(skus);
}

componentWillUnmount() {
  if (openBilling) {
    RNIap.endConnection();
  }
}

// component 2 => same as component 1
async componentDidMount() {
  if (!openBilling) {
    await RNIap.prepare();
  }
  const products = await RNIap.getProducts(skus);
}

componentWillUnmount() {
  if (openBilling) {
    RNIap.endConnection();
  }
}

Hope it helps you.

@axelkennedal
Copy link
Author

Yeah, that might be the beginning of a solution, but I think it needs to be paired with a mutex, otherwise there is still the chance that both components call prepare() at the same time (the second one manages to call it before the first one has finished and set the global variable). I'll see what I can do, though I think this library should handle this sort of thing built-in.

@hyochan
Copy link
Owner

hyochan commented May 9, 2018

@axelkennedal
Alright, I will consider your opinion. However, there is very very low chance(close to 0) to assign variable at the same time because once it is being assigned, it will be blocked from the memory based on computer science theory. Therefore, I guess above code will be working fine beyond your concern.

@axelkennedal
Copy link
Author

The way that you describe it there are two possible ways of interpreting it.

  1. Use a global variable ready which is false before any call to the library, and true once prepare() has finished running

OR

  1. Use a global variable openBilling which is false before any call to the library and set to true in the very beginning of the implementation of prepare()

For interpretation 1 this is possible to happen (c1 is a component and c2 is another component):

20180509_200218

And for interpretation 2 this can happen:

20180509_200816

And I'm sure there are more ways it can fail. It seems to me that your approach is too naive and doesn't work in all cases. When you have two components that load at the same time and try to interact with the library at the same time these things do happen (as I've seen when experimenting with it).

@axelkennedal
Copy link
Author

Here is a slightly more sophisticated solution, which handles the above sort of cases. I've tested it and it seems to work as intended.

What's the best way to structure the usage of react-native-iap?

The big problem is that on Android you need to wait for the function prepare() to be done before other interaction with the library can be made.

I've created my own class for managing In App Purchases, which builds upon react-native-iap.

Static and prepare()

My first thought was that I want my class to save as much fetched data as possible (for the current session), so that I don't perform multiple queries for the same thing if I've already fetched it. That's why I thought it would be a good idea to make the functions and variables of my class static, so that multiple React components could reuse the same data and not have to create a new object of my class for each component and fetch the same data multiple times just because it's used in multiple components. This would also make it so that I would only have to call prepare() once in my app (when it is foregrounded), and not before every single call to the library.

It turns out that if I call prepare() in componentDidMount() of App.js it doesn't get called before the other calls to the library are made (from other React components in my app). This means that some other call like getProducts(itemSKUs) gets called before prepare(), resulting in strange behaviour. Instead, calling prepare() in the constructor() of App.js works. Though the constructor can't be async and can't await the call to be finished, this doesn't matter since I am using a mutex to have the other calls to the class wait for the prepare() to be done. (So what matters here is that prepare() is called before all other calls to my class, which is guaranteed now that it is called from the constructor of the root component of my app).

The connection also needs to be ended at some point. A good time to do this is in componentWillUnmount() of App.js. This way a connected session will last the entire time that the app is in the foreground.

Mutex

It is possible to use an async-mutex and lock the mutex in my own prepare()-function and have all other functions of my class wait for the mutex to be unlocked before they interact with the react-native-iap-library. Before I found out about this package I used setInterval() to wait a second and hope it was done, which you really can't count on being the case (bad practice!).

A "gotcha" to look out for here is having one function which utilizes runExclusive() call another one which utilizes runExclusive(), since this will result in a deadlock: The first function starts running "exclusively" and makes a call to the second function, which in turn also calls runExclusive() - but since the first function is already being run "exclusively" it cannot run, and so it waits for the first function to finish - but the first function can't finish since it waits for the second function to finish.

Summary

  1. I call the prepare() function of my class in constructor() of App.js
  2. The implementation of my prepare() function locks a mutex, making all other calls wait for it to finish
  3. Other React components of my app load and some of them call functions on my class such as getProducts(itemSKUs)
  4. These calls "get in line" and wait for my prepare() function to finish
  5. prepare() finishes executing and releases the lock
  6. All waiting calls now start executing, one at a time

@hyochan
Copy link
Owner

hyochan commented May 10, 2018

@axelkennedal I exactly agree with what you've described above. I just posted the starter solution for your problem. It could be improved, like when you call getProducts and get failure message you can call it again after few seconds setting timeout method. Also, using mutex must be a better solution and for sure I should manage this in the native side. Will come back to you if I've done it.

@axelkennedal
Copy link
Author

I'm happy to hear you agree 😄 Yes, this should for sure be handled on the native side 👍

@JJMoon JJMoon added the 🤖 android Related to android label May 18, 2018
@hyochan
Copy link
Owner

hyochan commented May 20, 2018

@axelkennedal I've released 1.0.5 fixing this. When you called prepare method, then before you call endConnection it won't start billing client again. I'll close the issue. Please reopen it when you still have a problem in 1.0.5.

@hyochan hyochan closed this as completed May 20, 2018
@axelkennedal
Copy link
Author

I'm not sure I understand what you mean. Are you telling me that the other functions of the library will now wait until prepare has finished?

@hyochan
Copy link
Owner

hyochan commented May 20, 2018

@axelkennedal I've just blocked from starting new billing client in android when prepare is called multiple times. It will just start once. You should handle your needs in js side.

@nico1510
Copy link

@dooboolab do you know of any disadvantages of calling prepare just once for the whole application lifetime instead of before every API call ? I don't plan to use this library in components but in a redux saga which is running the whole time.

@hyochan
Copy link
Owner

hyochan commented Jun 1, 2018

@nico1510 Since billing service is running all the time, basically your application will assign another thread that lives all the time which is useless. Also, there maybe some defects in handling background service running in your app. It will be much safe to let go when you don't need that background service to be running.

@nico1510
Copy link

nico1510 commented Jun 2, 2018

@dooboolab thanks I'll follow your advice :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Related to android 🙏 help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants