xinbc / jdiameter

Automatically exported from code.google.com/p/jdiameter
0 stars 0 forks source link

Session based Client CCA/Gx/Ro throws Null Pointer Exceptions if send fails in HA mode in getGatheredRequestedAction() #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Configure stack in HA mode
2. Send a session based Ro CCR which fails (no server listening)
3. getGatheredRequestedAction() throws an NPE
4. If this is fixed, dispatch() throws an NPE because the session has been 
discarded before dispatch() is called.

What is the expected output? What do you see instead?
Requested Action is mandatory for Events, but is not required for session based 
messages. 
handleSendFailure() calls getGatheredRequestedAction() before deciding if the 
message is an Event or session based. This is fine in non-HA mode, but in HA it 
causes an NPE (as there is no Requested Action setGatheredRequestedAction() has 
not been called)

What version of the product are you using? On what operating system?
1.5.0 SNAPSHOT from git on CentOS 6.3.

Please provide any additional information below.
The solution to the first issue is simply to only call 
getGatheredRequestedAction() once we have decided that we have an Event. 
The second issue can be avoided by not removing the session when we go IDLE, 
however I believe this may cause us to leak sessions; the remove should be 
called a some point.

Attached is git diff showing my solutions. This probably needs a call to remove 
the session at some point. I'll look at that next week.

Original issue reported on code.google.com by andrew.m...@acision.com on 4 Jan 2013 at 5:55

Attachments:

GoogleCodeExporter commented 9 years ago
Will evaluate a proper fix. Andrew, let me know if you have made some progress 
on this already.

Thanks.

Original comment by brainslog on 11 Jan 2013 at 3:41

GoogleCodeExporter commented 9 years ago
The change from "setState(ClientRoSessionState.IDLE, true)" to 
"setState(ClientRoSessionState.IDLE, false)" in my original patch is wrong. 

Step 4 in the original bug report is not a real issue, the 
IllegalStateException can be caught at the application layer. My original patch 
does prevent the IllegalStateException, but causes other problems. I got 
carried away fixing exceptions...

The rest of the patch consists of moving gatheredRequestedAction and it's 
initialization inside the code block for Event messages. Within this block the 
absence of a Gathered Request Action is an error; outside this block it is not 
required and not used. 

I've attached the patch I am now using, which I believe is correct and safe, 
and fixes the real issue for me. 

Original comment by andrew.m...@acision.com on 11 Jan 2013 at 10:21

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks Andrew. I feel much more confortable with the new patch. Will check if 
it happens in other applications (probably CCA and a couple more) and apply it.

Original comment by brainslog on 11 Jan 2013 at 3:07

GoogleCodeExporter commented 9 years ago
The same issue also affects CCA and Gx.

Original comment by brainslog on 12 Jan 2013 at 6:09

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 1cd6476ff32c.

Fixed the issue in all affected applications and situations.

Original comment by brainslog on 12 Jan 2013 at 6:13