A (not so subtle) leak...
-
Since there's no horizontal scroll, your code leaks right off the edge of the screen. :cool:
Chris Meech I am Canadian. [heard in a local bar] Donate to help Conquer Cancer[^]
Sorry about that... Did not notice it (damn those 1920x1200 screens :-\ )
We are the all singing, all dancing crap of the world. - Tyler Durden
-
No. When the timeout occures the Receive() method does exit and returns -1 but the thread occupied by the BeginReceive() call does not exit. In other words it slowly (or fast depending on how often it used) fill's up all threads on the ThreadPool with nasty consequences for everything else depending on the ThreadPool. Changing
else
{
return -1;
}to
else
{
state.Socket.Close();
return -1;
}fixed it.
We are the all singing, all dancing crap of the world. - Tyler Durden
-
Sorry about that... Did not notice it (damn those 1920x1200 screens :-\ )
We are the all singing, all dancing crap of the world. - Tyler Durden
It's not the screen's fault.
-
In a project I am working on I had to do NTP time synchronisation on a WindowsCE client. The UdpClient it's Receive() call does not time out so I found this solution on the web:
public class UdpTimeoutClient : UdpClient { private class SocketState { public Socket Socket; public byte\[\] Buffer; public int BytesRead; public ManualResetEvent WaitHandle; public SocketState() { Socket = null; Buffer = null; BytesRead = 0; WaitHandle = new ManualResetEvent(false); } } private static void ReceiveCallback(IAsyncResult ar) { SocketState state = (SocketState)ar.AsyncState; try { state.BytesRead = state.Socket.EndReceive(ar); } catch { } state.WaitHandle.Set(); } public int Receive(ref byte\[\] buffer, int timeout) { SocketState state; AsyncCallback callback; IAsyncResult result; state = new SocketState(); state.Socket = Client; state.Buffer = buffer; callback = new System.AsyncCallback(UdpTimeoutClient.ReceiveCallback); result = state.Socket.BeginReceive(state.Buffer, 0, state.Buffer.Length, System.Net.Sockets.SocketFlags.None, callback, state); if (state.WaitHandle.WaitOne(timeout, false)) { return state.BytesRead; } else { return -1; } } }Can you spot the leak?
We are the all singing, all dancing crap of the world. - Tyler Durden
-
Jan van den Baard wrote:
Can you spot the leak?
Why use an asynchronous approach when you handle it synchronously?
xacc.ide - now with IronScheme support
IronScheme - 1.0 alpha 1 out nowTo be able to timeout from receiving the data. The method should be handled synchronously but it should also be able to timeout. When I would use the synchronous Receive() call on the UdpClient it would wait indefinitly for data to arive. I.E. the call would never exit. This way I can atleast timeout when receiving expected data takes to long. Our NTP client does a request on the NTP server at regular intervals. The NTP server may not be available all the time. If I where to use the Receive() approach the NTP syncing would "hang" after the first failed synchronisation attempt.
We are the all singing, all dancing crap of the world. - Tyler Durden
-
In a project I am working on I had to do NTP time synchronisation on a WindowsCE client. The UdpClient it's Receive() call does not time out so I found this solution on the web:
public class UdpTimeoutClient : UdpClient { private class SocketState { public Socket Socket; public byte\[\] Buffer; public int BytesRead; public ManualResetEvent WaitHandle; public SocketState() { Socket = null; Buffer = null; BytesRead = 0; WaitHandle = new ManualResetEvent(false); } } private static void ReceiveCallback(IAsyncResult ar) { SocketState state = (SocketState)ar.AsyncState; try { state.BytesRead = state.Socket.EndReceive(ar); } catch { } state.WaitHandle.Set(); } public int Receive(ref byte\[\] buffer, int timeout) { SocketState state; AsyncCallback callback; IAsyncResult result; state = new SocketState(); state.Socket = Client; state.Buffer = buffer; callback = new System.AsyncCallback(UdpTimeoutClient.ReceiveCallback); result = state.Socket.BeginReceive(state.Buffer, 0, state.Buffer.Length, System.Net.Sockets.SocketFlags.None, callback, state); if (state.WaitHandle.WaitOne(timeout, false)) { return state.BytesRead; } else { return -1; } } }Can you spot the leak?
We are the all singing, all dancing crap of the world. - Tyler Durden
(Note: in addition to the problem you found.) You don't have a Dispose method, implement IDisposable, and you don't dispose of Socket nor WaitHandle. It will eventually fall through to the objects' finalizers, but that will happen at an unpredictable time in the future. Meanwhile your object, and any objects it's referencing, is hanging around on the heap. GC deals with objects that require finalization by putting them on a list of objects to be finalized - the freachable queue (for 'finalizable objects still reachable'.) Then, another thread processes this queue. The memory can only be freed once the GC runs a second time. Unfortunately it also tunes to run less frequently if more objects survive a GC. Also, if you're deriving from UdpClient, you already have a socket. Unfortunately you can't access it from a derived class. Unless you really need to be able to dynamically substitute a UdpTimeoutClient for a UdpClient, I'd remove the inheritance link and make it a standalone class. Actually, I can see no reason that UdpClient was left unsealed.
DoEvents: Generating unexpected recursion since 1991
-
(Note: in addition to the problem you found.) You don't have a Dispose method, implement IDisposable, and you don't dispose of Socket nor WaitHandle. It will eventually fall through to the objects' finalizers, but that will happen at an unpredictable time in the future. Meanwhile your object, and any objects it's referencing, is hanging around on the heap. GC deals with objects that require finalization by putting them on a list of objects to be finalized - the freachable queue (for 'finalizable objects still reachable'.) Then, another thread processes this queue. The memory can only be freed once the GC runs a second time. Unfortunately it also tunes to run less frequently if more objects survive a GC. Also, if you're deriving from UdpClient, you already have a socket. Unfortunately you can't access it from a derived class. Unless you really need to be able to dynamically substitute a UdpTimeoutClient for a UdpClient, I'd remove the inheritance link and make it a standalone class. Actually, I can see no reason that UdpClient was left unsealed.
DoEvents: Generating unexpected recursion since 1991
Mike Dimmick wrote:
You don't have a Dispose method, implement IDisposable, and you don't dispose of Socket nor WaitHandle.
Unless I am mistaking the Dispose() of UdpClient does the necessary cleanup for me.
Mike Dimmick wrote:
Also, if you're deriving from UdpClient, you already have a socket. Unfortunately you can't access it from a derived class.
Well you can. Through the Client property. The class uses the socket supplied by this property. Disposing of the UdpTimeoutClient object should perform all necessary cleanup. Not sure about the WaitHandle though...
-
In a project I am working on I had to do NTP time synchronisation on a WindowsCE client. The UdpClient it's Receive() call does not time out so I found this solution on the web:
public class UdpTimeoutClient : UdpClient { private class SocketState { public Socket Socket; public byte\[\] Buffer; public int BytesRead; public ManualResetEvent WaitHandle; public SocketState() { Socket = null; Buffer = null; BytesRead = 0; WaitHandle = new ManualResetEvent(false); } } private static void ReceiveCallback(IAsyncResult ar) { SocketState state = (SocketState)ar.AsyncState; try { state.BytesRead = state.Socket.EndReceive(ar); } catch { } state.WaitHandle.Set(); } public int Receive(ref byte\[\] buffer, int timeout) { SocketState state; AsyncCallback callback; IAsyncResult result; state = new SocketState(); state.Socket = Client; state.Buffer = buffer; callback = new System.AsyncCallback(UdpTimeoutClient.ReceiveCallback); result = state.Socket.BeginReceive(state.Buffer, 0, state.Buffer.Length, System.Net.Sockets.SocketFlags.None, callback, state); if (state.WaitHandle.WaitOne(timeout, false)) { return state.BytesRead; } else { return -1; } } }Can you spot the leak?
We are the all singing, all dancing crap of the world. - Tyler Durden
result = state.Socket.BeginReceive(state.Buffer, 0, state.Buffer.Length, System.Net.Sockets.SocketFlags.None, callback, state); looks potentially recursive, behaviour problem in parent class UdpClient?
MrPlankton
-
In a project I am working on I had to do NTP time synchronisation on a WindowsCE client. The UdpClient it's Receive() call does not time out so I found this solution on the web:
public class UdpTimeoutClient : UdpClient { private class SocketState { public Socket Socket; public byte\[\] Buffer; public int BytesRead; public ManualResetEvent WaitHandle; public SocketState() { Socket = null; Buffer = null; BytesRead = 0; WaitHandle = new ManualResetEvent(false); } } private static void ReceiveCallback(IAsyncResult ar) { SocketState state = (SocketState)ar.AsyncState; try { state.BytesRead = state.Socket.EndReceive(ar); } catch { } state.WaitHandle.Set(); } public int Receive(ref byte\[\] buffer, int timeout) { SocketState state; AsyncCallback callback; IAsyncResult result; state = new SocketState(); state.Socket = Client; state.Buffer = buffer; callback = new System.AsyncCallback(UdpTimeoutClient.ReceiveCallback); result = state.Socket.BeginReceive(state.Buffer, 0, state.Buffer.Length, System.Net.Sockets.SocketFlags.None, callback, state); if (state.WaitHandle.WaitOne(timeout, false)) { return state.BytesRead; } else { return -1; } } }Can you spot the leak?
We are the all singing, all dancing crap of the world. - Tyler Durden
any updates regarding above issue? :)
-
any updates regarding above issue? :)
Yes. See my response to the message from AWdrius above.
We are the all singing, all dancing crap of the world. - Tyler Durden
-
any updates regarding above issue? :)
The OP said what was wrong up above.
The best way to accelerate a Macintosh is at 9.8m/sec² - Marcus Dolengo