@eyedeekay
&eche|on
&zzz
+Irc2PGuest81845
+R4SAS
+RN
+StormyCloud
+cumlord
+dr|z3d
+eche|off
+mareki2p
+orignal
+postman
+qend-irc2p
+snex
Arch2
Birdy
BubbRubb
Chrono
Daddy
Dann
DeltaOreo
Irc2PGuest27269
Irc2PGuest28007
Irc2PGuest34181
Irc2PGuest53924
Irc2PGuest5682
Irc2PGuest65801
Irc2PGuest74254
Irc2PGuest90985
Irc2PGuest92550
Irc2PGuest95708
Onn4l7h
Onn4|7h
Over
Sisyphus
Sleepy
SlippyJoe
Stormycloud_
T3s|4_
Teeed
aargh2
ac9f_
acetone_
anontor
b3t4f4c3__
dr4wd3
duanin2
duck
eyedeekay_bnc
gellegery
leopold_
makoto
matean
n1
nilbog
not_bob_afk
null912
onon_
poriori
profetikla
r00tobo_BNC
shiver_
solidx66
u5657
uop23ip
w8rabbit
x74a6
eyedeekay
zzz I missed them I think connectivity problems my bouncer doesn't have them either
zzz
hoo boy, stand by for big pasta
eyedeekay
Fire away
zzz
<dr|z3d> Error processing job [Timeout OB Client Message Lease Lookup] on thread 8
zzz
<dr|z3d> java.lang.NullPointerException: Cannot invoke "net.i2p.router.client.ClientConnectionRunner.getFloodfillNetworkDatabaseFacade()" because "runner" is null
zzz
<dr|z3d> at net.i2p.router.client.ClientManager.getClientFloodfillNetworkDatabaseFacade(ClientManager.java:798)
zzz
<dr|z3d> at net.i2p.router.client.ClientManagerFacadeImpl.getClientFloodfillNetworkDatabaseFacade(ClientManagerFacadeImpl.java:306)
zzz
<dr|z3d> at net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseSegmentor.getSubNetDB(FloodfillNetworkDatabaseSegmentor.java:92)
zzz
<dr|z3d> at net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseSegmentor.clientNetDB(FloodfillNetworkDatabaseSegmentor.java:285)
zzz
<dr|z3d> at net.i2p.router.RouterContext.clientNetDb(RouterContext.java:378)
zzz
<dr|z3d> at net.i2p.router.message.OutboundClientMessageOneShotJob$LookupLeaseSetFailedJob.runJob(OutboundClientMessageOneShotJob.java:592)
zzz
<dr|z3d> at net.i2p.router.JobQueueRunner.runCurrentJob(JobQueueRunner.java:118)
zzz
<dr|z3d> at net.i2p.router.JobQueueRunner.run(JobQueueRunner.java:67)
zzz
<zzz> yeah dr|z3d eyedeekay the first one is gonna happen for out-of-session lookups, that code was my headscratcher from the other day
zzz
<zzz> lets look at this one
zzz
<dr|z3d> getting several of thse as well: Error processing job [Timeout OB Client Message Lease Lookup] on thread 2
zzz
<dr|z3d> java.lang.NullPointerException
zzz
<zzz> yeah that one is a flat-out bug
zzz
<zzz> he didn't remove the useless null check I pointed out, and I missed the null problem in the next line
zzz
<dr|z3d> so we're not checking if runner == null there.
zzz
<dr|z3d> this is all still pretty perplexing, so the best I can do is report errors where I find them.
zzz
<zzz> oh wait I was looking in wrong place
zzz
<dr|z3d> public FloodfillNetworkDatabaseFacade getClientFloodfillNetworkDatabaseFacade(Hash destHash) {
zzz
<dr|z3d> if (destHash != null) {
zzz
<dr|z3d> if (_log.shouldLog(Log.DEBUG))
zzz
<dr|z3d> _log.debug("Getting subDb for desthash: " + destHash);
zzz
<dr|z3d> ClientConnectionRunner runner = getRunner(destHash);
zzz
<dr|z3d> if (_log.shouldLog(Log.DEBUG))
zzz
<dr|z3d> _log.debug("ClientManager got a runner in getClientFloodfillNetworkDatabaseFacade for " + destHash);
zzz
<dr|z3d> return runner.getFloodfillNetworkDatabaseFacade();
zzz
<dr|z3d> }
zzz
<dr|z3d> return null;
zzz
<dr|z3d> }
zzz
<zzz> yeah
zzz
<zzz> the next method getCFNDFacade*s*() has different bugs
zzz
<dr|z3d> should we be falling back to mainNetDb?
zzz
<zzz> yes, but imho the fallback code should be (solely) in Segmentor. this method should be able to return null
zzz
<dr|z3d> ok
zzz
<dr|z3d> on the upside, they're CRITICAL bugs, so they won't be overlooked.
zzz
<zzz> so part of my heartburn is he's also doing fallback in ClientConnectionRunner.getFloodfillNetworkDatabaseFacade() which dunno why
zzz
<zzz> but the ClientManager bugs are just basic null check bugs
zzz
<dr|z3d> so something like?
zzz
<dr|z3d> if (runner !== null) {
zzz
<dr|z3d> return runner.getFloodfillNetworkDatabaseFacade();
zzz
<dr|z3d> } else {
zzz
<dr|z3d> if (_log.shouldLog(Log.WARN))
zzz
<dr|z3d> _log.warn("Runner in getClientFloodfillNetworkDatabaseFacade not available for " + destHash);
zzz
<dr|z3d> }
zzz
<zzz> sure.. here's the way I'd do it (untested) to fix both methods
zzz
<zzz> --- a/router/java/src/net/i2p/router/client/ClientManager.java
zzz
<zzz> +++ b/router/java/src/net/i2p/router/client/ClientManager.java
zzz
<zzz> @@ -785,6 +785,8 @@ class ClientManager {
zzz
<zzz> if (_log.shouldLog(Log.DEBUG))
zzz
<zzz> _log.debug("Getting subDb for desthash: " + destHash);
zzz
<zzz> ClientConnectionRunner runner = getRunner(destHash);
zzz
<zzz> + if (runner == null)
zzz
<zzz> + return null;
zzz
<zzz> if (_log.shouldLog(Log.DEBUG))
zzz
<zzz> _log.debug("ClientManager got a runner in getClientFloodfillNetworkDatabaseFacade for " + destHash);
zzz
<zzz> return runner.getFloodfillNetworkDatabaseFacade();
zzz
<zzz> @@ -800,8 +802,9 @@ class ClientManager {
zzz
<zzz> public Set<FloodfillNetworkDatabaseFacade> getClientFloodfillNetworkDatabaseFacades() {
zzz
<zzz> Set<FloodfillNetworkDatabaseFacade> rv = new HashSet<FloodfillNetworkDatabaseFacade>();
zzz
<zzz> for (ClientConnectionRunner runner : _runners.values()) {
zzz
<zzz> - if (runner != null)
zzz
<zzz> - rv.add(runner.getFloodfillNetworkDatabaseFacade());
zzz
<zzz> + FloodfillNetworkDatabaseFacade fndf = runner.getFloodfillNetworkDatabaseFacade();
zzz
<zzz> + if (fndf != null)
dr|z3d
works fine here.
zzz
<zzz> + rv.add(fndf);
zzz
<zzz> }
zzz
<zzz> return rv;
zzz
<zzz> }
zzz
<zzz> I assume you're hitting these on your box that does pings?
zzz
<dr|z3d> no, standard router, nothing special happening on it.
zzz
<zzz> hmph
zzz
<dr|z3d> possibly related to i2pchat. all I can think of.
zzz
<zzz> just wondering how we can help eyedeekay improve his testing process
zzz
<dr|z3d> I've suggested i2pchat might be helpful, it seems to have surfaced bugs in the past, if not this one.
zzz
<zzz> is i2pchat a SAM client?
zzz
<dr|z3d> yeah
zzz
EOT
zzz
dr|z3d, please confirm, you mean my fix works?
dr|z3d
yes indeed, appears to be causing no issues.
zzz
super, thanks
eyedeekay
Ack ok I got all that
eyedeekay
Thanks for the scrollback
zzz
np
zzz
couple suggestions:
zzz
1) be a lot more careful/deliberate about null checks when you're banging out code. In 15 lines of code you have 3 bugs: two places where you should have had it and one where you didn't need it.
zzz
2) It's clients, or fairly transient clients, that are triggering whole bunches of bugs (of course because that's what creates subdbs), so try to develop some better tests so you catch the bugs before they get to drz
zzz
more clients or some SAM torture test or something? just spitballing
zzz
EOT
eyedeekay
+1 will work on it/come up with something
zzz
e.g. fire up 100 SAMs. Do lookups on half, do servers on the other half; kill them all after 5 minutes, and loop
eyedeekay
Ack can do