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

Overhaul internal resource management #1563

Merged
merged 10 commits into from
Jan 30, 2025
Merged

Overhaul internal resource management #1563

merged 10 commits into from
Jan 30, 2025

Conversation

d4rken
Copy link
Member

@d4rken d4rken commented Jan 29, 2025

A rewrite of SharedResource, which is a class that manages the lifecycle of costly dependencies. These costly services are mostly Root and ADB related (launching a service in an extra process and connecting to it).

Replacing RXShell (#1529) made flaws within the old SharedResource visible, where we could trigger a race condition if one consumer gets a new lease, while another closes the last one.

I couldn't get the Flow.shareIn logic (on which the old SharedResource was based) to work. This is custom solution which gives me finer control over the behavior.

This should get us

  • Less overhead when sharing resources
  • No more random errors related to root or ADB 🤞
  • Less resource use due to better re-use of the shared resource
  • Now we have test coverage for this part

@d4rken d4rken added enhancement New feature, request, improvement or optimization c: IO SAF/Normal/Root access Root ADB ADB/Shizuku related labels Jan 29, 2025
I just couldn't get rid of the race condition when starting/stopping the `shareIn` Flow
Fix race condition we new lease was not yet added, but leaseCheck ran and cancelled the sourceJob despite an ongoing get() call

Simplify debug flags

Refactor debug flags

Fix test

Fix flaky test

New debug flags

Improve debug output

Further work on SharedResource + more tests

Fixed race-conditions related to fast and short lifecycles get()+close()

There might still be an issue related to addChild(...).close and InternalCancellationException being thrown?

Fix null sourceValue being accessed on scope cancellation
@d4rken d4rken force-pushed the task-start-hanging branch from fc3a54d to 70c8202 Compare January 30, 2025 13:54
@d4rken d4rken marked this pull request as ready for review January 30, 2025 14:30
@d4rken d4rken merged commit 86507b5 into main Jan 30, 2025
16 checks passed
@d4rken d4rken deleted the task-start-hanging branch January 30, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADB ADB/Shizuku related c: IO SAF/Normal/Root access enhancement New feature, request, improvement or optimization Root
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant