Opened 3 years ago
Last modified 23 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 3 years ago by
Status: | new → open |
---|
comment:2 Changed 3 years ago by
Milestone: | undecided → 0.9.33 |
---|
I searched and could only find two places in the code where we call rebuildRouterInfo(true).
- FloodfillMonitorJob?, where we switch from non-ff to ff, or vice versa. Will never happen on Android.
- FloodfillNetworkDatabaseFacade?,shutdown(), when we are floodfill. Will never happen on Android.
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 3 years ago by
Component: | router/general → apps/android |
---|---|
Owner: | set to str4d |
Status: | open → assigned |
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 3 years ago by
@str4d please fix for .34 if wasn't fixed in .33, please update this ticket
comment:5 Changed 23 months ago by
Milestone: | 0.9.33 → 0.9.42 |
---|---|
Owner: | changed from str4d to Meeh |
Worth researching why it's the way it is. netdb.publish(), when inline, can take a while, especially with 1800+ floodfills to choose from.