Hello Sebastian, I catch NPE while debugging :( Clean just imported DB was used. The NPE was 100% reproducible on my server
I'll doublecheck On Dec 27, 2012 5:25 AM, "[email protected]" <[email protected]> wrote: > In fact by reading this method, logically your if-statement should never > be true. > > Cause: > the method "sendUploadCompletMessageByPublicSID" first tries to get the > RoomClient with: > RoomClient currentClient = this.clientListManager > .getClientByPublicSID(publicSID, false, > null); > > The null at the end means it searches a RoomClient with that publicSID and > the serverId = null. That means it searches for a Connection/Session > LOCALLY. > > If RoomClient is != null => Then it can directly use the regular sync > methods. Cause it means that the session is handled on the same server. > If tRoomClient is NULL, then it means it is handled on a slave server. > I have impelemented this so that the master can sync the upload complete > message to the slave. > > What you have implemented then at the end with that if-clause, actually > the implementation logically can never run into that. > If the serverId == null, the RoomClient would have already be found by the > first getClientByPublicSID(publicSID, false, null); > > From my point of view an Exception should here be thrown and we should > find out how and when it could happen that the "server" argument is null > here. Cause actually it would mean a much bigger issue. > > Where did you find any kind of issue that makes you think that this > if-clause is needed? > > Sebastian > > > > > > > 2012/12/27 [email protected] <[email protected]> > >> Hi Maxim, >> >> I found this fix from you: >> >> Server s = clientSessionInfo.getServerId() != null ? >> serverDao.get(clientSessionInfo.getServerId()) : null; >> if (s != null) { >> // no need to sync on slave if server is null >> clusterSlaveJob.syncMessageToClientOnSlave(s, >> clientSessionInfo.getRcl().getPublicSID() , message); >> } >> >> What should that mean and what enhancements should it bring? >> >> Actually if server == null it means that the client is handled on the >> same server. >> Basically on a slave ALL sessions have the server == null, because from >> the perspective of the slave every session is locally. In fact the slave >> does not even know that he is a slave. He handles every connection as if >> there is no difference. >> >> So why should the slave NOT sync that message ? That makes no sense to me. >> >> Server == null is a correct implementation and it should not throw any >> NullPointerException. >> It simply means that the Session is local and not on another server. >> Actually only this kind of session could have a s != null: >> A session that is synced from the slave to the master. The master would >> have this session with a Server != null. >> >> So your comment does not makes sense to me. >> Of course slaves do sync messages. On the slave the "server" argument is >> _always_ null. But of course the slave should still sync that message. >> >> Sebastian >> >> >> -- >> Sebastian Wagner >> https://twitter.com/#!/dead_lock >> http://www.webbase-design.de >> http://www.wagner-sebastian.com >> [email protected] >> > > > > -- > Sebastian Wagner > https://twitter.com/#!/dead_lock > http://www.webbase-design.de > http://www.wagner-sebastian.com > [email protected] >
