Skip to content

Move all fragment work to UI thread #356

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

Conversation

newyankeecodeshop
Copy link
Contributor

Summary

Our app crashed with the following call stack:

java.util.ConcurrentModificationException
  at java.util.HashMap$HashIterator.nextEntry(HashMap.java:851)
  at java.util.HashMap$ValueIterator.next(HashMap.java:879)
  at androidx.fragment.app.FragmentStore.findFragmentByTag(FragmentStore.java:233)
  at androidx.fragment.app.FragmentManager.findFragmentByTag(FragmentManager.java:1677)
  at com.reactcommunity.rndatetimepicker.RNTimePickerDialogModule.open(RNTimePickerDialogModule.java:99)
  at java.lang.reflect.Method.invoke(Method.java:-2)
  at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
  at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:151)
  at com.facebook.react.bridge.queue.NativeRunnable.run(NativeRunnable.java:-2)
  at android.os.Handler.handleCallback(Handler.java:751)
  at android.os.Handler.dispatchMessage(Handler.java:95)
  at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
  at android.os.Looper.loop(Looper.java:154)
  at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:226)
  at java.lang.Thread.run(Thread.java:761)

It would appear to be caused by this code running at the same time the UI thread is manipulating the map of fragments. Given that the code that adds/updates fragments must run on the UI thread, it would seem that code that queries for fragments also needs to use the UI thread.

Test Plan

Used the updated code in our app and made sure the time picker dialog functions properly.

What's required for testing (prerequisites)?

An Android app that shows the date and time pickers.

What are the steps to reproduce (after prerequisites)?

I was not able to reproduce it given that it involves two threads manipulating fragments at the same time.

Compatibility

OS Implemented
iOS N/A
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)
  • I have added automated tests, either in JS or e2e tests, as applicable

@vonovak vonovak merged commit 267a20f into react-native-datetimepicker:master Dec 4, 2020
@vonovak
Copy link
Member

vonovak commented Dec 4, 2020

thank you for the PR!

@newyankeecodeshop newyankeecodeshop deleted the fix-concurrent-mod-exception branch December 4, 2020 14:43
vonovak pushed a commit that referenced this pull request Dec 4, 2020
## [3.0.8](v3.0.7...v3.0.8) (2020-12-04)

### Bug Fixes

* move all fragment work to UI thread ([#356](#356)) ([267a20f](267a20f))
@vonovak
Copy link
Member

vonovak commented Dec 4, 2020

🎉 This PR is included in version 3.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@msschwartz
Copy link

To help other devs down the road if they run across this error. This is the error message I get using any version before 3.0.8: Method addObserver must be called on the main thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants