Hi George,
Synchronizing the PendingRequest.cancel() method causes deadlock
problems. The source problem is the early object release
in that method. The retry creation phase can be protected by
a simple flag that is true (on) while the next retry is being
prepared and not yet sent.
Please replace the PendingRequest class in Snmp.java with the
code below (or the next nightly build) and let me know if
the NPE goes away:
Best regards,
Frank
class PendingRequest extends TimerTask implements PduHandleCallback {
private PduHandle key;
protected int retryCount;
protected ResponseListener listener;
protected Object userObject;
protected PDU pdu;
protected Target target;
protected TransportMapping transport;
private int requestStatus = 0;
// Maximum request status - allows to receive up to two reports and
then
// send the original request again. A value of 0 is used for discovery.
private int maxRequestStatus = 2;
private volatile boolean finished = false;
private volatile boolean responseReceived = false;
private volatile boolean pendingRetry = false;
public PendingRequest(ResponseListener listener,
Object userObject,
PDU pdu,
Target target,
TransportMapping transport) {
this.userObject = userObject;
this.listener = listener;
this.retryCount = target.getRetries();
this.pdu = pdu;
this.target = (Target) target.clone();
this.transport = transport;
}
private PendingRequest(PendingRequest other) {
other.pendingRetry = true;
this.userObject = other.userObject;
this.listener = other.listener;
this.retryCount = other.retryCount - 1;
this.pdu = other.pdu;
this.target = other.target;
this.requestStatus = other.requestStatus;
this.responseReceived = other.responseReceived;
this.transport = other.transport;
}
protected void registerRequest(PduHandle handle) {
}
public void responseReceived() {
this.responseReceived = true;
}
public synchronized void pduHandleAssigned(PduHandle handle, Object
pdu) {
if (key == null) {
key = handle;
pendingRequests.put(handle, this);
registerRequest(handle);
if (logger.isDebugEnabled()) {
logger.debug("Running pending "+
((listener instanceof SyncResponseListener) ?
"sync" : "async")+
" request with handle " + handle+
" and retry count left "+retryCount);
}
long delay = timeoutModel.getRetryTimeout(target.getRetries() -
retryCount,
target.getRetries(),
target.getTimeout());
if ((!finished) && (!responseReceived)) {
timer.schedule(this, delay);
}
}
}
/**
* Process retries of a pending request.
*/
public synchronized void run() {
PduHandle m_key = key;
PDU m_pdu = pdu;
Target m_target = target;
TransportMapping m_transport = transport;
ResponseListener m_listener = listener;
Object m_userObject = userObject;
if ((m_key == null) || (m_pdu == null) || (m_target == null) ||
(m_listener == null)) {
if (logger.isDebugEnabled()) {
logger.debug("PendingRequest canceled key="+m_key+", pdu="+m_pdu+
", target="+m_target+", transport="+m_transport+",
listener="+
m_listener);
}
return;
}
try {
boolean retry;
synchronized (pendingRequests) {
retry = (!finished) && (retryCount > 0) && (!responseReceived);
}
if (retry) {
try {
PendingRequest nextRetry = new PendingRequest(this);
sendMessage(m_pdu, m_target, m_transport, nextRetry);
this.pendingRetry = false;
}
catch (IOException ex) {
finished = true;
logger.error("Failed to send SNMP message to " + m_target +
": " +
ex.getMessage());
messageDispatcher.releaseStateReference(m_target.getVersion(),
m_key);
listener.onResponse(new ResponseEvent(Snmp.this, null,
m_pdu, null,
m_userObject,
ex));
}
}
else if (!finished) {
finished = true;
pendingRequests.remove(m_key);
if (!responseReceived) {
// request timed out
if (logger.isDebugEnabled()) {
logger.debug("Request timed out: " +
m_key.getTransactionID());
}
messageDispatcher.releaseStateReference(m_target.getVersion(),
m_key);
listener.onResponse(new ResponseEvent(Snmp.this, null,
m_pdu, null,
m_userObject));
}
}
else {
// make sure pending request is removed even if response listener
// failed to call Snmp.cancel
pendingRequests.remove(m_key);
}
}
catch (RuntimeException ex) {
logger.error("Failed to process pending request " + m_key +
" because " + ex.getMessage(), ex);
throw ex;
}
catch (Error er) {
logger.fatal("Failed to process pending request " + m_key +
" because " + er.getMessage(), er);
throw er;
}
}
public boolean setFinished() {
boolean currentState = finished;
this.finished = true;
return currentState;
}
public void setMaxRequestStatus(int maxRequestStatus) {
this.maxRequestStatus = maxRequestStatus;
}
public int getMaxRequestStatus() {
return maxRequestStatus;
}
public boolean isResponseReceived() {
return responseReceived;
}
/**
* Cancels the request and clears all internal fields by setting them
* to <code>null</code>.
* @return
* <code>true</code> if cancellation was successful.
*/
public boolean cancel(){
boolean result = super.cancel();
// free objects early
if (!pendingRetry) {
key = null;
pdu = null;
target = null;
transport = null;
listener = null;
userObject = null;
}
return result;
}
}
George Tsaloukidis wrote:
Dear Frank,
I am using snmp4j version 1.9.3d for executing synchronous snmp GET requests.
My application has many threads, but each thread uses its own org.snmp4j.Snmp
instance.
Sometimes I get the following exception:
java.lang.NullPointerException
at org.snmp4j.Snmp$PendingRequest.pduHandleAssigned(Unknown Source)
at org.snmp4j.MessageDispatcherImpl.sendPdu(Unknown Source)
at org.snmp4j.Snmp.sendMessage(Unknown Source)
at org.snmp4j.Snmp$PendingRequest.run(Unknown Source)
at java.util.TimerThread.mainLoop(Timer.java:512)
at java.util.TimerThread.run(Timer.java:462)
Exception in thread "Timer-4877612" java.lang.NullPointerException
at org.snmp4j.Snmp$PendingRequest.pduHandleAssigned(Unknown Source)
at org.snmp4j.MessageDispatcherImpl.sendPdu(Unknown Source)
at org.snmp4j.Snmp.sendMessage(Unknown Source)
at org.snmp4j.Snmp$PendingRequest.run(Unknown Source)
at java.util.TimerThread.mainLoop(Timer.java:512)
at java.util.TimerThread.run(Timer.java:462)
After rebuilding SNMP4J.jar with debug information I saw that the problem was
at line 1449 of Snmp.java file:
long delay = timeoutModel.getRetryTimeout(target.getRetries() -
retryCount,
target.getRetries(),
target.getTimeout());
That means the target object was null while trying to retransmit the request.
That can happen only for two reasons: a) PendingRequest of the retry action
created
just after the cancellation of the very first PendingRequest (line 1487) or,
the PendingRequest canceled while inside the pduHandleAssigned() method.
The second case can appear only if the main thread gets that object from the
pendingRequests list (line 811).
That means the Timer thread had previously managed to execute the 1440 line and
suspended for a while.
The problem is that the send() method (line 793) can call the cancel()
operation on a PendingRequest
at any time, without taking into consideration if the TimerThread is executed.
A solution can be to first lock the PendingRequest before calling setFinished()
and cancel() methods.
Line 811 should also go after the cancellation of the first request or else if
PendingRequest.run() has started,
the newly created PendingRequest is left into the pendingRequests list for ever.
I have tested the above solution and I had no problem.
And something else. At line 810 the wait() method is called.
Sun advices such calls to be wrapped into a while loop
(see http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.8.1 and
http://java.sun.com/javase/6/docs/api/java/lang/Object.html#wait() links)
while(syncResponse.response == null)
{
syncResponse.wait();
}
Best regards,
George Tsaloukidis,
Senior Software Engineer
Systems Software Developement
Intracom Telecom S.A
_______________________________________________
SNMP4J mailing list
[email protected]
http://lists.agentpp.org/mailman/listinfo/snmp4j
--
AGENT++
http://www.agentpp.com
http://www.mibexplorer.com
http://www.mibdesigner.com
_______________________________________________
SNMP4J mailing list
[email protected]
http://lists.agentpp.org/mailman/listinfo/snmp4j
