Opened 18 months ago

Last modified 15 months ago

#2096 assigned defect

ANR when getting RouterInfo

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

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 (add)

Change History (4)

comment:1 Changed 18 months ago by zzz

  • Status changed from new to open

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 18 months ago by zzz

  • Milestone changed from undecided to 0.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 18 months ago by zzz

  • Component changed from router/general to apps/android
  • Owner set to str4d
  • Status changed from open to 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 15 months ago by zzz

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

Note: See TracTickets for help on using tickets.