Opened 2 years ago

Last modified 6 months ago

#2096 assigned defect

ANR when getting RouterInfo

Reported by: str4d Owned by: Meeh
Priority: minor Milestone: 0.9.42
Component: apps/android Version: 0.9.31
Keywords: I2P Android, hang Cc:
Parent Tickets: Sensitive: no

Description

Seen on various Android devices:

"main" prio=5 tid=1 Blocked
  | group="main" sCount=1 dsCount=0 obj=0x7626f000 self=0xb4827800
  | sysTid=14023 nice=-11 cgrp=default sched=0/0 handle=0xb6f56bec
  | state=S schedstat=( 0 0 0 ) utm=811 stm=143 core=1 HZ=100
  | stack=0xbe784000-0xbe786000 stackSize=8MB
  | held mutexes=
  at net.i2p.router.Router.getRouterInfo (Router.java:519)
- waiting to lock <0x20b029bd> (a java.lang.Object) held by thread 85
  at net.i2p.android.router.util.Util.getNetStatus (Util.java:436)
  at net.i2p.android.router.MainFragment.updateStatus (MainFragment.java:359)
  at net.i2p.android.router.MainFragment.access$600 (MainFragment.java:49)
  at net.i2p.android.router.MainFragment$OneShotUpdate.run (MainFragment.java:270)

The thread holding the lock isn't shown in any of the reports, but I'm guessing it is a call to Router.rebuildRouterInfo(blockingRebuild=true).

Would it make sense to move the publishing of the RouterInfo? outside the lock? Publishing happens after a call to Router.setRouterInfo(ri), at which point other callers to Router.getRouterInfo() should be able to proceed fine (and the publisher is one such caller).

Subtickets

Change History (5)

comment:1 Changed 2 years ago by zzz

Status: newopen

Worth researching why it's the way it is. netdb.publish(), when inline, can take a while, especially with 1800+ floodfills to choose from.

comment:2 Changed 2 years ago by zzz

Milestone: undecided0.9.33

I searched and could only find two places in the code where we call rebuildRouterInfo(true).

However, even when blockingRebuild == false, it may take quite a while, as it calls commSystem.createAddresses() which has locks of its own and may hae a lot of work to do.

In Router, I'll change the locking to a read/write lock, but that won't help if it's stuck in createAddresses().

I think the fix for you is to just remove the call to get the router info, and the detailed checks for what's in the NTCP and SSU addresses in Util.getNetStatus(). This looks like code copied from console SummaryHelper?.reachability(). A lot of the special cases we put into specific error messages in the console aren't really necessary in Android. It's a simpler UI with much less configuration options, a lot less ability for the user to misconfigure something, and less facilities to fix something that's broken.

Now that we have a lot of states that can get returned from commSystem.getStatus(), that should be granular enough without needing to inspect the RouterInfo? contents. If not, we could consider adding yet more states to getStatus().

comment:3 Changed 2 years ago by zzz

Component: router/generalapps/android
Owner: set to str4d
Status: openassigned

Changed to a r/w lock in 821022ec07265c32461b5106f2a72b0ef00c2c61 to be 0.9.32-15.

Reassigning to OP to look at Android Util.getNetStatus().

comment:4 Changed 22 months ago by zzz

@str4d please fix for .34 if wasn't fixed in .33, please update this ticket

comment:5 Changed 6 months ago by zzz

Milestone: 0.9.330.9.42
Owner: changed from str4d to Meeh
Note: See TracTickets for help on using tickets.