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

Android: Fix crash in handleDroppedPeers #270

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

ovitrif
Copy link
Contributor

@ovitrif ovitrif commented Sep 6, 2024

Fixes synonymdev/bitkit#2133

(Android-only) Fix to avoid ConcurrentModificationException while addedPeers list is modified from other thread.

Former setup:

  1. Scheduling was done using TimerTask which creates a different thread each time it's invoked
  2. Since we also change addedPeers from react-native, there was a chance to get into this issue especially at app start, when both the react-native code as well as the native LDKModule code would mutate the same list.

Fix:
Use a thread-safe mutable list implementation, I picked ConcurrentLinkedQueue because it's less resource-consuming than other options.

Extra:
Rewrote the implementation of the scheduled repeating task, to use more performant and adequate APIs which actually wait for the scheduled work to finish, then delay the next execution with 3 seconds.


@ovitrif ovitrif force-pushed the fix/2133-handle-dropped-peers-crash branch 2 times, most recently from 361a9ec to 6475a79 Compare September 6, 2024 13:32
@ovitrif ovitrif force-pushed the fix/2133-handle-dropped-peers-crash branch 2 times, most recently from 021ff8d to 1c0879c Compare September 6, 2024 14:12
@ovitrif ovitrif force-pushed the fix/2133-handle-dropped-peers-crash branch from 1c0879c to 981c124 Compare September 6, 2024 14:13
@Jasonvdb Jasonvdb merged commit ebb1440 into master Sep 9, 2024
3 of 6 checks passed
@Jasonvdb Jasonvdb deleted the fix/2133-handle-dropped-peers-crash branch September 9, 2024 06:57
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.

[Bug]: Crash shortly after opening App (Android)
2 participants