by Sebastian Andersson
posted on 2004/09/14 |
|
.NET has the same memory model as Java, which (among other things) means that memory reads/writes do not have to happen in the same order as in the code and they don't have to be transfered from one thread's view of the memory to another thread's memory view; data can be cached in processor's registers, in temporary variables etc as the environment sees fit and the order of read and write may happen in any order. lock() has to be used if one wants to be sure data is transfered from one thread to another. There are plenty of Java articles that describe this far better than I can, search google on "java memory model".
One bug of this kind I've seen on a SMP machine is in Ssl.AsyncResult there a deadlock sometimes happen. When the deadlock has happened, one thread is stuck doing WaitOne() on the ManualResetEvent from AsyncResult. The ManualResetEvent's m_Completed field is set to true, so no code will call Set on the ManualResetEvent (m_WaitHandle) object. I assume that both the Notify method and the AsyncWaitHandle get property have run at the same time by two threads on two processors. I assume this causes the the environment somewhere to delay the writing of m_Completed and/or the m_WaitHandle, which it is allowed to do according to the memory model of .Net and thus the other thread fails to either see that m_WaitHandle has been set (the if(m_WaitHandle != null) test fails in Notify) or fails to see that m_Completed is true (the if(m_Completed) fails). In both cases, m_WaitHandle is never Set and the caller will block forever.
Either a lock has to be used to protect the access to the m_Completed field or the ManualResetEvent will always have to be created (so both methods are sure to see it/use it). I changed a local copy to always create the ManualResetEvent, which seems to solve it for me. |
by Gabe Halsmer [gabe dot halsmer at attbi dot com]
posted on 2004/10/02 |
|
Yes I got the deadlocks too. I fixed it by simplifying the code a little. I was using SecureSocket's regular Accept, Send, and Receive, which internally are all based on the Async callback.
Whoever designed that probably was wanting to get some code reuse. However, it made the code way more complicated then it needed to be.
Here's a rewrite of SecureSocket.Accept...
System.Net.Sockets.Socket socket = this.InternalAccept();
return new SecureSocket(socket, this.m_Options);
And for Send...
if (SecureProtocol == SecureProtocol.None)
return base.Send(buffer, offset, size, socketFlags);
else
{
buffer = this.m_Controller.m_RecordLayer.EncryptBytes(buffer, offset, size, ContentType.ApplicationData);
size = buffer.Length;
return this.m_Controller.m_Socket.Send(buffer, offset, size, socketFlags);
}
Receive was the hardest to rewrite without the Async, but actually I think that's were the deadlock bug was.
First comment out the call to m_Socket.BeginReceive in SocketContoller's constructor. Then modify SecureSocket's Receive to this...
int count = 0;
XBuffer decryptBuffer = this.m_Controller.m_DecryptedBuffer;
if (decryptBuffer.Length > 0)
{
// There may be decrypted bytes left-over from the previous call to Receive.
// If there's enough to complete this request, then we don't need to
// call Receive on the socket.
int moveBytes = Math.Min( size, (int)decryptBuffer.Length );
decryptBuffer.Seek(0, System.IO.SeekOrigin.Begin);
decryptBuffer.Read(buffer, offset, moveBytes);
decryptBuffer.RemoveXBytes(moveBytes);
count += moveBytes;
if (count > 0)
return count;
}
byte[] temporaryBuffer = new byte[65500];
System.Net.Sockets.Socket socket = this.m_Controller.m_Socket;
while (count == 0)
{
int bytesReceived = socket.Receive(temporaryBuffer, 0, temporaryBuffer.Length, SocketFlags.None);
SslRecordStatus status;
if (this.m_Controller.m_RecordLayer == null)
{
CompatibilityResult ret = this.m_Controller.m_Compatibility.ProcessHello(temporaryBuffer, 0, bytesReceived);
this.m_Controller.m_RecordLayer = ret.RecordLayer;
status = ret.Status;
if (this.m_Controller.m_RecordLayer != null)
this.m_Controller.m_Compatibility = null;
}
else
{
status = this.m_Controller.m_RecordLayer.ProcessBytes(temporaryBuffer, 0, bytesReceived);
}
if (status.Buffer != null)
{
if (status.Status == SslStatus.Close)
{
// shut down the connection after the send
this.m_Controller.m_IsShuttingDown = true;
}
socket.Send(status.Buffer, 0, status.Buffer.Length, SocketFlags.None);
}
else if (status.Status == SslStatus.Close)
{
// Record Layer instructs us to shut down
socket.Shutdown(SocketShutdown.Both);
}
else if (status.Status == SslStatus.OK)
{
}
if (status.Decrypted != null)
{
// copy status.Decrypted to controller's buffer
decryptBuffer.Seek(0, System.IO.SeekOrigin.End);
decryptBuffer.Write(status.Decrypted, 0, status.Decrypted.Length);
// copy as many bytes as needed from the contoller's buffer
// to our output buffer. We may be left with extra bytes in the
// controller's buffer. They'll be used in the next Receive.
int remainingBytes = size - count;
int moveBytes = Math.Min( remainingBytes, (int)decryptBuffer.Length );
decryptBuffer.Seek(0, System.IO.SeekOrigin.Begin);
decryptBuffer.Read(buffer, offset + count, moveBytes);
decryptBuffer.RemoveXBytes(moveBytes);
count += moveBytes;
}
}
return count;
|
