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