[SNMP4J] Why 'stop' on SocketException in DefaultUdpTransportMapping.ListenThread?
Frank Fock
fock at agentpp.com
Wed May 1 01:02:02 CEST 2013
Kelly,
Yes, the change wasn't a good one. It does not work if the exception is
thrown permantly, for example if the socket was closed (although
this should not happen). You can improve the behavior by
adding the following method to DefaultUdpTransportMapping:
/**
* If receiving new datagrams fails with a {@link SocketException},
this method is called to renew the
* socket - if possible.
* @param socketException
* the exception that occurred.
* @return
* the new socket or <code>null</code> if the listen thread should
be terminated with the provided
* exception.
* @throws SocketException
* a new socket exception if the socket could not be renewed.
*/
protected DatagramSocket renewSocketAfterException(SocketException
socketException) throws SocketException {
DatagramSocket s = new DatagramSocket(udpAddress.getPort());
s.setSoTimeout(socketTimeout);
return s;
}
Then change the DefaultUdpTransportMapping.ListenThread.run() method as
follows:
public void run() {
DatagramSocket socketCopy = socket;
if (socketCopy != null) {
try {
socketCopy.setSoTimeout(getSocketTimeout());
if (receiveBufferSize > 0) {
socketCopy.setReceiveBufferSize(Math.max(receiveBufferSize,
maxInboundMessageSize));
}
if (logger.isDebugEnabled()) {
logger.debug("UDP receive buffer size for socket " +
getAddress() + " is set to: " +
socketCopy.getReceiveBufferSize());
}
} catch (SocketException ex) {
logger.error(ex);
setSocketTimeout(0);
}
}
while (!stop) {
DatagramPacket packet = new DatagramPacket(buf, buf.length,
udpAddress.getInetAddress(),
udpAddress.getPort());
try {
socketCopy = socket;
try {
if (socketCopy == null) {
stop = true;
continue;
}
socketCopy.receive(packet);
}
catch (InterruptedIOException iiox) {
if (iiox.bytesTransferred <= 0) {
continue;
}
}
if (logger.isDebugEnabled()) {
logger.debug("Received message from "+packet.getAddress()+"/"+
packet.getPort()+
" with length "+packet.getLength()+": "+
new OctetString(packet.getData(), 0,
packet.getLength()).toHexString());
}
ByteBuffer bis;
// If messages are processed asynchronously (i.e. multi-threaded)
// then we have to copy the buffer's content here!
if (isAsyncMsgProcessingSupported()) {
byte[] bytes = new byte[packet.getLength()];
System.arraycopy(packet.getData(), 0, bytes, 0, bytes.length);
bis = ByteBuffer.wrap(bytes);
}
else {
bis = ByteBuffer.wrap(packet.getData());
}
TransportStateReference stateReference =
new
TransportStateReference(DefaultUdpTransportMapping.this, udpAddress, null,
SecurityLevel.undefined,
SecurityLevel.undefined,
false, socketCopy);
fireProcessMessage(new UdpAddress(packet.getAddress(),
packet.getPort()), bis,
stateReference);
}
catch (SocketTimeoutException stex) {
// ignore
}
catch (PortUnreachableException purex) {
synchronized (DefaultUdpTransportMapping.this) {
listener = null;
}
logger.error(purex);
if (logger.isDebugEnabled()) {
purex.printStackTrace();
}
if (SNMP4JSettings.isFowardRuntimeExceptions()) {
throw new RuntimeException(purex);
}
break;
}
catch (SocketException soex) {
if (!stop) {
logger.warn("Socket for transport mapping " + toString() +
" error: " + soex.getMessage());
}
if (SNMP4JSettings.isFowardRuntimeExceptions()) {
stop = true;
throw new RuntimeException(soex);
}
else {
try {
DatagramSocket newSocket = renewSocketAfterException(soex);
if (newSocket == null) {
throw soex;
}
socket = newSocket;
} catch (SocketException e) {
stop = true;
logger.error("Socket renewal for transport mapping " +
toString() +
" failed with: " + e.getMessage(), e);
}
}
}
catch (IOException iox) {
logger.warn(iox);
if (logger.isDebugEnabled()) {
iox.printStackTrace();
}
if (SNMP4JSettings.isFowardRuntimeExceptions()) {
throw new RuntimeException(iox);
}
}
}
synchronized (DefaultUdpTransportMapping.this) {
listener = null;
stop = true;
DatagramSocket closingSocket = socket;
if ((closingSocket != null) && (!closingSocket.isClosed())) {
closingSocket.close();
}
socket = null;
}
if (logger.isDebugEnabled()) {
logger.debug("Worker task stopped:" + getClass().getName());
}
}
Best regards,
Frank
Am 30.04.2013 23:05, schrieb Kelly Dyer:
> I am a little confused by this change.
>
> We have run into this issue, with a sporadic socket closed exception, the cause of which we have not been able to identify. What we are seeing is that the new behavior results in a busy-loop, since we go right back to a receive that is guaranteed to fail.
>
> This strikes me as a case where we need to either remediate the socket directly, or cancel the listener. What am I missing?
>
> Regards,
> Kelly.
>
>
> On 12.07.2011 14:02, Fock, Frank wrote:
>> Hi Gary,
>>
>> Your right, the current behavior is too strict.
>> I have changed it as follows for the 2.0 release
>> (will also change it for the 1.x branch too):
>>
>> catch (SocketException soex) {
>> if (!stop) {
>> logger.error("Socket for transport mapping " + toString() +
>> " error: " + soex.getMessage(), soex);
>> }
>> if (SNMP4JSettings.isFowardRuntimeExceptions()) {
>> stop = true;
>> throw new RuntimeException(soex);
>> }
>> }
>>
>> Best regards,
>> Frank
>>
>> On 11.07.2011 16:29, Smith, Gary wrote:
>>> Folks
>>>
>>> I'm using SNMP4j 1.11.3 and get occasional socket exceptions from certain devices, but not all.
>>>
>>> Current DefaultUdpTransportMapping.ListenThread.run() behaviour is to stop listening if any SocketException is caught (line 394).
>>>
>>> This kills the listener thread for every device, not just the offending device.
>>>
>>> I have a badly behaved/config'ed device that is provoking an exception:
>>>
>>> Socket for transport mapping org.snmp4j.transport.DefaultUdpTransportMapping$ListenThread at 1264c84 error: socket closed
>>> java.net.SocketException: socket closed
>>> at java.net.PlainDatagramSocketImpl.receive0(Native Method)
>>> at java.net.PlainDatagramSocketImpl.receive(PlainDatagramSocketImpl.java:136)
>>> at java.net.DatagramSocket.receive(DatagramSocket.java:712)
>>> at org.snmp4j.transport.DefaultUdpTransportMapping$ListenThread.run(Unknown Source)
>>> at java.lang.Thread.run(Thread.java:619)
>>>
>>>
>>> This effectively kills the ListenThread for all my polled devices - by removing the "stop = true;" line above, my application becomes resilient (although obviously I have some > config problems).
>>>
>>>
>>> Thanks
>> Gary
>>> _______________________________________________
>>> SNMP4J mailing list
>>> SNMP4J at agentpp.org
>>> http://lists.agentpp.org/mailman/listinfo/snmp4j
> _______________________________________________
> SNMP4J mailing list
> SNMP4J at agentpp.org
> http://lists.agentpp.org/mailman/listinfo/snmp4j
--
---
AGENT++
Maximilian-Kolbe-Str. 10
73257 Koengen, Germany
https://agentpp.com
Phone: +49 7024 8688230
Fax: +49 7024 8688231
More information about the SNMP4J
mailing list