Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

thaliMobile needs clean up native Android peer listeners when it declares a peer unavailable #1454

Open
yaronyg opened this issue Oct 28, 2016 · 1 comment
Assignees

Comments

@yaronyg
Copy link
Member

yaronyg commented Oct 28, 2016

When thaliMobile accepts that a peer is no longer available it needs to call the thaliMobileNativeWrapper.terminateListener for that peer so we don't squat on sockets forever.

@chapko
Copy link
Contributor

chapko commented Mar 20, 2017

From the thaliMobileNative.js

 * @property {boolean} peerAvailable If true this indicates that the peer is
 * available for connectivity. If false it means that the peer can no longer be
 * connected to. For too many reasons to count it's perfectly possible to never
 * get a false for peerAvailable. It is also possible to get a false when the
 * peer is still reachable. A classic example is on Android where the app can go
 * into the background reducing the power to the BLE radio which can make the
 * peer seem to disappear. But Bluetooth would still be on full power so a
 * connect could still work. So this value can at best be treated as a hint.

If this is still the case (possible to get a false when the peer is still reachable) then cleaning up peer listener will make it impossible to connect to the peer again even if it is still available.

I see 2 options here:

  1. We always trust peerAvaible: false from native layer and when we receive it we clean up peer listeners.
  2. We implement Update notification for iOS (aka get rid of recreated peerAvailabilityChanged events) #1527 and then it doesn't matter if the peer is still reachable, because peer listener will be created on demand (by getPeerHostInfo call) and not when peerAvailabilityChanged event is received.

The first option is very easy and simple but it is just a temporary solution because #1527 should be implemented anyway and it will change everything.

Initially I wanted to suggest the first option, but since #1832 has been discovered I think it is time to re-prioritize #1527.

@chapko chapko removed their assignment Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants