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: Random crash with map view in Ti.UI.ListSection footer #13243

Closed
drauggres opened this issue Jan 14, 2022 · 43 comments · Fixed by #13263
Closed

Android: Random crash with map view in Ti.UI.ListSection footer #13243

drauggres opened this issue Jan 14, 2022 · 43 comments · Fixed by #13263

Comments

@drauggres
Copy link
Contributor

https://jira.appcelerator.org/browse/TIMOB-28583

Exception
E AndroidRuntime: FATAL EXCEPTION: main
E AndroidRuntime: Process: ti.map.test, PID: 29094
E AndroidRuntime: java.lang.IllegalArgumentException: No view found for id 0x2 (unknown) for fragment SupportMapFragment{ab59c34} (12aebe3e-06b7-46e3-b9ca-2d53d2ec238d id=0x2)
E AndroidRuntime:        at androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:513)
E AndroidRuntime:        at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:282)
E AndroidRuntime:        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2189)
E AndroidRuntime:        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2100)
E AndroidRuntime:        at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2002)
E AndroidRuntime:        at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:524)
E AndroidRuntime:        at android.os.Handler.handleCallback(Handler.java:873)
E AndroidRuntime:        at android.os.Handler.dispatchMessage(Handler.java:99)
E AndroidRuntime:        at android.os.Looper.loop(Looper.java:193)
E AndroidRuntime:        at android.app.ActivityThread.main(ActivityThread.java:6669)
E AndroidRuntime:        at java.lang.reflect.Method.invoke(Native Method)
E AndroidRuntime:        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
E AndroidRuntime:        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
App sample
const mapView = Ti.UI.createView({
  height: 200
});
mapView.add(require('ti.map').createView());

const win = Ti.UI.createWindow({
  title: 'test',
  backgroundColor: 'gray'
});

function addListView() {
  var listView = Ti.UI.createListView({
    footerView: mapView,
    templates: {
      test: {
        childTemplates: [{
          type: 'Ti.UI.View',
          childTemplates: [{
            type: 'Ti.UI.Label',
            bindId: 'label',
            properties: {
              color: 'black',
              bindId: 'label'
            }
          }],
          properties: {
            width: Ti.UI.FILL,
            height: 200,
            cardUseCompatPadding: true,
            backgroundColor: 'white'
          }
        }]
      }
    },
    defaultItemTemplate: 'test'
  });
  var section = Ti.UI.createListSection();
  var items = [];

  for (let i = 0; i < 20; i++) {
    items.push({
      label: {
        text: 'item ' + i
      },
      template: 'test'
    })
  }

  section.setItems(items);
  listView.sections = [section];
  win.add(listView);
}

win.addEventListener('open', function() {
  addListView();
})
win.open();

Original discussion

I think the reason is a race condition between FragmentTransaction and attaching view to a window, i.e. app will crash when the transaction is executed before the view has been attached.

P.S. somehow connecting to the app with debugger make it harder to reproduce the problem.

@m1ga
Copy link
Contributor

m1ga commented Jan 15, 2022

maybe this can help to debug a bit more: https://stackoverflow.com/questions/42205827/add-fragment-in-recyclerview-viewholder

@hansemannn
Copy link
Collaborator

I hereby offer a bounty for fixing this issue in the SDK and/or ti.map. Is there anyone able to hack on this?

@drauggres
Copy link
Contributor Author

Doesn't the trick with postlayout work?

@hansemannn
Copy link
Collaborator

It does work so far, but the rendering is (visibly) delayed and iOS lags as well, causing a platform-specific implementation of the map. A fixed Android version would eliminate those issues.

@hansemannn
Copy link
Collaborator

Actually, it still rarely crashes with the same error E/com.appc.test Invalid ID 0x00000005.

@m1ga
Copy link
Contributor

m1ga commented Jan 31, 2022

@hansemannn can you please try this one:
ti.map-android-5.3.5.zip
I moved the TiUIMapView to extends TiUIView. My test app with a footerView works fine.

@hansemannn
Copy link
Collaborator

Thanks Michael! Is there a commit I could cherry pick? We use a highly adjusted version of ti.map 😗

@m1ga
Copy link
Contributor

m1ga commented Jan 31, 2022

Sure: https://github.com/m1ga/ti.map/tree/testing
I didn't do any tests besides opening the app. Had to remove the override at interceptTouchEvent

@hansemannn
Copy link
Collaborator

Will test asap already! Note for later cleanup: Rename layout_lottie with layout_map 😇

@m1ga
Copy link
Contributor

m1ga commented Jan 31, 2022

yes 😄 At the moment I'm even not sure if the XML is needed at all. Was testing multiple stuff at once 🤣

@hansemannn
Copy link
Collaborator

Unfortunately I cannot test it, yet, because it collides with ti.animation:

[ERROR] TiExceptionHandler: Error: Attempt to invoke virtual method 'void com.airbnb.lottie.LottieAnimationView.setScaleType(android.widget.ImageView$ScaleType)' on a null object reference

Likely because of the same XML file? Will test later then!

@m1ga
Copy link
Contributor

m1ga commented Jan 31, 2022

oh interesting. I've updated the name

@hansemannn
Copy link
Collaborator

hansemannn commented Jan 31, 2022

Interesting, thank you! Now it renders, but none of the properties are applied anymore (like annotations, mapStyle, padding etc.). EDIT: And sometimes it now doesn't render at all.

@m1ga
Copy link
Contributor

m1ga commented Jan 31, 2022

ok, but it looks like we can change ti.map instead of stuff in the SDK 👍 Makes it a bit quicker to debug.

@drauggres
Copy link
Contributor Author

ok, but it looks like we can change ti.map instead of stuff in the SDK Makes it a bit quicker to debug.

I don't think so. Internally, you still use a Fragment, meaning that the problem (adding two views asynchronously) still there. (I could be wrong. I never really worked with Fragments)

A possible solution would be:

  • addind new "view-attached-to-window"-event
  • passing it through all child-views when a view is attached (to a window directly or to another view that has already been attached)
  • listening for this event in TiUIFragment and executing the transaction when the event is received.

But I'm not sure how it will affect regular views (performance-wise).

@hansemannn
Copy link
Collaborator

@drauggres Very interested in that approach! Another one could be https://developers.google.com/maps/documentation/android-sdk/lite which may be enough because the map does not have any interaction required

@m1ga
Copy link
Contributor

m1ga commented Feb 2, 2022

@hansemannn liteMode is already supported (2 years 😉 ) but it didn't change anything.

@hansemannn
Copy link
Collaborator

Then it has to be the recyclerview itself. Worst case, it would need a static generated image..This now really shows the Titanium limits.

drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 3, 2022
prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix tidev#13243
@drauggres
Copy link
Contributor Author

drauggres commented Feb 3, 2022

@hansemannn please try this patch:
drauggres@14859b8

Though it does not crash, I'm not sure about memory leaks.
Probably we should also override onDetachedFromWindow and do something, idk.

UPD: I mean, if there was a memory leak, this patch does not fix it, but it shouldn't create more "leakage".

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

🤣 just doing some tests here too:
Screenshot_20220203_130355

I'll test it right away, too!

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

Looking very good 👍 My test app doesn't crash!

@hansemannn
Copy link
Collaborator

hansemannn commented Feb 3, 2022

Whaaat? Super excited here! @drauggres Will try to test your solution as well!

@hansemannn
Copy link
Collaborator

@drauggres Unfortunately still crashing sporadically :(

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

with the same No view found for id 0x2 (unknown) error?

@hansemannn
Copy link
Collaborator

hansemannn commented Feb 3, 2022

It's always E/com.appc.app Invalid ID 0x0000000a. for us. Before, it was E/com.appc.app Invalid ID 0x00000006., indicating that 3 views are added prior to the mapview now (0x00000007, 0x00000008 and 0x00000009) if Titanium always counts app. Still using the postlayout hack right now by the way. I will try to remove that one.

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

ah ok, the "no view" part is the next line.

E/com.miga.rando: Invalid ID 0x00000002.
TiExceptionHandler: (main) [2590,2701] No view found for id 0x2 (unknown) for fragment SupportMapFragment{59a6fe9} (043b23b5-a8ef-435e-8bd6-0af1a9922127 id=0x2)
TiExceptionHandler:
TiExceptionHandler:     androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:513)
TiExceptionHandler:     androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:282)
TiExceptionHandler:     androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2189)
TiExceptionHandler:     androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2100)
TiExceptionHandler:     androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2002)
TiExceptionHandler:     androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:524)
TiExceptionHandler:     android.os.Handler.handleCallback(Handler.java:938)
TiExceptionHandler:     android.os.Handler.dispatchMessage(Handler.java:99)
TiExceptionHandler:     android.os.Looper.loopOnce(Looper.java:201)
TiExceptionHandler:     android.os.Looper.loop(Looper.java:288)
TiExceptionHandler:     android.app.ActivityThread.main(ActivityThread.java:7839)
TiExceptionHandler:     java.lang.reflect.Method.invoke(Native Method)
TiExceptionHandler:     com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
TiExceptionHandler:     com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

@drauggres
Copy link
Contributor Author

@hansemannn Please look for stacktrace in logcat.

@hansemannn
Copy link
Collaborator

Checking! Never saw the above trace actually. But maybe crashlytics grabs it away, which I can't think of. Will check with adb logcat.

@hansemannn
Copy link
Collaborator

When I remove the postlayout hack, it currently seems to work..No annoations are rendered, but that may be something else. Still on it using the patch from @drauggres

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

don't use my patched ti.map version! With the normal ti.map I had annoations and click working

@hansemannn
Copy link
Collaborator

hansemannn commented Feb 3, 2022

I don't. Having no changes on ti.map right now, but it could be a migration issue from moving back to the non-postlayout config. EDIT: NOT related to this change (!) Has to be something from the migration back. So the patch looks VERY solid right now!

EDIT 2: Annotation issue was a client-side issue. Testing with a wider range of devs now. Still looks promising, @drauggres!

@hansemannn
Copy link
Collaborator

hansemannn commented Feb 3, 2022

Okay, so the crashes are now gone completely. What we do notice is that when opening the map view in non-listviews (e.g. own window), the map is always stuck at the equator line, which works 1:1 when changing the SDK version back. It may be fixable by timeout, because we currently use timeouts as well which may need to wait a bit longer now.

EDIT: Still crashing on Android 7 here, but newer versions seem to be fine. Not sure how that could come, but we do see an improvement.

EDIT 2: The only glitch remaining is that the map is now redrawn every time a user scrolls down and up again. We still have to verify if thats only during your chance, but it's probably a whole other issue.

EDIT 3: Confirmed: The redrawing is not happening with the old version. The new version also removes all polylines, which is very odd. The liteMode may help for that, but I am unsure how your change could have caused this.

@drauggres
Copy link
Contributor Author

My guest is that you set location and do other api calls before map is actually created.

For example:
https://github.com/appcelerator-modules/ti.map/blob/a12d8d34d3a7c7cd672d9142775a304279de0a88/android/src/ti/map/ViewProxy.java#L1329-L1337

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

liteMode has other issues with the patch. But I can confirm that the polyline is gone and there is a refresh. But my region (set at creation) stays

@hansemannn
Copy link
Collaborator

The region can be set with a small timeout (50ms) which works then, but the flickering is really odd.

@drauggres
Copy link
Contributor Author

https://github.com/drauggres/ti.map.tests

This app based on code from android/example folder from ti.map module.
I added case with ListView to "Drawing" tests.
Don't see any problems in either original or modified test with patched SDK.

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

Nice test app.
If you scroll up again and then down it will refresh the map and the polygons are cone.

Multimap crashed for me the first time:

[ERROR] TiExceptionHandler: (main) [2985,3631] com.google.maps.api.android.lib6.impl.ar cannot be cast to android.view.ViewGroup
[ERROR] TiExceptionHandler:
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:504)
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:282)
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2189)
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2100)
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2002)
[ERROR] TiExceptionHandler:     androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:524)
[ERROR] TiExceptionHandler:     android.os.Handler.handleCallback(Handler.java:938)
[ERROR] TiExceptionHandler:     android.os.Handler.dispatchMessage(Handler.java:99)
[ERROR] TiExceptionHandler:     android.os.Looper.loopOnce(Looper.java:201)
[ERROR] TiExceptionHandler:     android.os.Looper.loop(Looper.java:288)
[ERROR] TiExceptionHandler:     android.app.ActivityThread.main(ActivityThread.java:7839)
[ERROR] TiExceptionHandler:     java.lang.reflect.Method.invoke(Native Method)
[ERROR] TiExceptionHandler:     com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
[ERROR] TiExceptionHandler:     com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

2nd time I could open it (I opened other items before).

@drauggres
Copy link
Contributor Author

Multimap crashed for me the first time:

Yes, it also crashing in 10.1.1.GA.

@m1ga
Copy link
Contributor

m1ga commented Feb 3, 2022

ah, wait. That issue is here: tidev/ti.map#480 with a "fix" inside.

@drauggres
Copy link
Contributor Author

If you scroll up again and then down it will refresh the map and the polygons are cone.

On the bright side, now it works more correctly: it frees resources. lol

drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 3, 2022
execution transaction on every "onAttachedToWindow" cause loss of objects on map

refs tidev#13243
drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 3, 2022
execution transaction on every "onAttachedToWindow" causes loss of objects on map

refs tidev#13243
@drauggres
Copy link
Contributor Author

drauggres commented Feb 3, 2022

master...drauggres:bug/map_in_listview

UPD: this should fix the problem with polylines.

drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 8, 2022
…ew being attached to a window

prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix tidev#13243
drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 8, 2022
…iew being attached to a window

prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix tidev#13243
drauggres added a commit to drauggres/titanium_mobile that referenced this issue Feb 8, 2022
…iew being attached to a window

prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix tidev#13243
@hansemannn
Copy link
Collaborator

hansemannn commented Feb 18, 2022

Quick update here: I thought we fixed it already by removing some duplicate re-rendering, but it sometimes happens again now. I will test your PR in more detail now! :) Do you know by any chance if the TiUIFragment is used for other APIs as well that could suffer from regressions?

Btw: I will reach out to you the next weeks for the promised bounty! Didn't forget that - really thankful for your help here! 🚀

hansemannn pushed a commit that referenced this issue Mar 21, 2022
…iew being attached to a window (#13263)

prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix #13243
@hansemannn hansemannn reopened this Apr 15, 2022
@hansemannn
Copy link
Collaborator

hansemannn commented Apr 15, 2022

I have to reopen this one :( We still get some crashes:

java.lang.IllegalArgumentException: 
  at androidx.fragment.app.FragmentStateManager.createView (FragmentStateManager.java:513)
  at androidx.fragment.app.FragmentStateManager.moveToExpectedState (FragmentStateManager.java:282)
  at androidx.fragment.app.FragmentManager.executeOpsTogether (FragmentManager.java:2189)
  at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute (FragmentManager.java:2100)
  at androidx.fragment.app.FragmentManager.execPendingActions (FragmentManager.java:2002)
  at androidx.fragment.app.FragmentManager$5.run (FragmentManager.java:524)
  at android.os.Handler.handleCallback (Handler.java:808)
  at android.os.Handler.dispatchMessage (Handler.java:101)
  at android.os.Looper.loop (Looper.java:166)
  at android.app.ActivityThread.main (ActivityThread.java:7529)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:245)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:921)

A similar crash was reported for RN as well: software-mansion/react-native-screens#463 and it seems like it could be something related to the "FragmentTransaction" class and how it is reused (which would make sense in a ListView context). Happy to sponsor any work on this fix again!

EDIT: Added it via a new issue in #13399!

drauggres added a commit to drauggres/titanium_mobile that referenced this issue Jul 23, 2023
prevent crash when using ti.map inside a ListView

ref TIMOB-28583 fix tidev#13243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants