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

Multiple instances of EmbeddedBrowserActivity can exist and run at the same time #122

Open
dianabarsan opened this issue Oct 8, 2020 · 4 comments
Labels
Help wanted Good for first time contributions Type: Bug Fix something that isn't working as intended

Comments

@dianabarsan
Copy link
Member

dianabarsan commented Oct 8, 2020

The medic-android main activity, EmbeddedBrowserActivity does not have any launch mode set. This means that multiple instances of this activity can exist in parallel.
Each of these instances will connect to the server, replicate and consume device resources. Each of these roughly equate to having the app opened in multiple tabs in the same browser.

We have no control over these tasks, we do not manage them in any capacity and do not prevent them from being created.
Users also do not have control, once "in the background" such a task is not accessible, is ignored by the app and continues to function until the app is killed and started again.

I've seen multiple instances being started with regular use a few times in development, but so scarcely that I didn't investigate. But if another app would create an intent to launch the medic-android app, this would happen on a regular basis.

To determine whether these background instances exist, I:

  • connected my device to my computer and watched chrome://inspect/#devices
  • launch medic-android app
  • observe the inspect page, looking for the app's name (the name is the android application ID) and check how many activities there are (note: I think the service worker always gets its own activity, so that one doesn't count, it's also removed once the service worker's job is done)
  • create an intent in another app to launch a new EmbeddedBrowserActivity (by triggering the StartupActivity) and launched this intent
  • see a new activity appear in my inspect page. Navigating in the app only updates one of the activity URLs
  • repeating launching the intent always resulted in a new activity

To test whether these activities are indeed "active", I used the phone as the single client for my local server (so all incoming requests would be from this device).
I logged in with an admin account on the device and, locally, directly through couch (bypassing API), I edited a random doc.
Because the device, as an admin account, would listen for changes, I would expect to see the existent longpoll changes request be answered and immediately see a new one get created. So my log would show one REQ and one RES.

Doing such, this is what my logs looked like when I had one activity:

RES ed035e8e-6dbf-428b-958f-df2444b413c5 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182263 ..... <truncated>
REQ 689491f3-37c5-4b00-97d1-c206c0561098 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182264-... <truncated>

When I had 2 activities:

RES 689491f3-37c5-4b00-97d1-c206c0561098 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182264...<truncated>
RES 0758d716-f7dd-4929-9466-7be940a6ae1e ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182264...<truncated>
REQ 492201af-77de-4b6c-905f-d3f6a64e0292 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182265...<truncated>
REQ ebda2c7b-3d5f-423f-b230-8a66a6c9b779 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182265...<truncated>

When I had 3 activities:

RES 72938d46-6458-484a-8a95-4683dc7f42d0 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182266...
RES 19f41800-70d4-4cc2-9c66-24ae8656dd80 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182266...
RES 5b18fae9-9e80-4844-bf7b-efb487ee0bc0 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182266....
REQ c81514f7-5c88-455f-802a-5f43be3557fa ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182267...
REQ b0681274-8c52-49bc-9a7b-65f032fdd84d ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182267...
REQ 5b96208d-bd83-489f-8ef2-84a28b6675b4 ::ffff:79.113.159.99 - GET /medic/_changes?feed=longpoll&heartbeat=10000&since=182267...

I have observed that the background tasks were much slower to react than the one on top, and it could even take up to 10 seconds to get all of these requests, but got them without fault, every time.

I think we should change our manifest and add some launchmode to our EmbeddedBrowserActivity (or to our StartupActivity) to make sure we won't get into this situation.
Given that I remember seeing multiple activities in the past, without launching them intentionally, makes me think that this could happen in production and could have been happening for a while. Prevalence is undetermined, but we could probably scour some logs to see identical requests comming from the same ip at roughly the same time.

@dianabarsan dianabarsan added the Type: Bug Fix something that isn't working as intended label Oct 8, 2020
@garethbowen
Copy link
Member

Added to 3.11 because it should be a quick win and doesn't conflict with the angular upgrade. This is not a blocker for the release.

@garethbowen
Copy link
Member

Delayed to 3.12.0 to speed up 3.11.0 release.

@garethbowen garethbowen added the Help wanted Good for first time contributions label Feb 9, 2021
@garethbowen garethbowen added this to the 0.9.0 - DRAFT milestone Jun 2, 2021
@mrsarm mrsarm modified the milestones: 0.9.0, 0.10.0 Aug 4, 2021
@mrsarm mrsarm modified the milestones: 0.10.0, 0.11.0 Sep 14, 2021
@mrsarm mrsarm modified the milestones: 0.11.0, 1.0.0 Nov 25, 2021
@garethbowen
Copy link
Member

Not a blocker for 1.0.0

@garethbowen garethbowen removed this from the 1.0.0 milestone Mar 15, 2022
@latin-panda
Copy link
Contributor

latin-panda commented Mar 21, 2022

It'd be good to adopt some new practices now that we have modernised our libraries, like changing the activity class that these classes extend to another that has more functionality (FragmentActivity, AppCompatActivity, etc), then we can use callback contracts when intents are resolved. Sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted Good for first time contributions Type: Bug Fix something that isn't working as intended
Projects
None yet
Development

No branches or pull requests

4 participants