Skip to content

Feature/optimize 01 Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffer lists #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jogibear9988 merged 2 commits into from
Oct 7, 2026
Merged

Feature/optimize 01 Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffer lists #11

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41a926a
Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffe…
Stiwa-PohG Sep 11, 2026
626fe21
Update OpenSSLConnector.cs
Stiwa-PohG Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 63 additions & 21 deletions src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class OpenSSLConnector : IDisposable
private readonly byte[] m_buffer = new byte[4096];
private readonly DataBufferList m_pendingWriteList;
private readonly DataBufferList m_pendingReadList;
private readonly object m_readListLock = new object();
private readonly object m_writeListLock = new object();
Comment on lines +36 to +37

@Stiwa-PohG Stiwa-PohG Oct 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possible solution in the class OpenSSLConnector

Add one connector-wide gate object, e.g. m_sslLock, and make every entry point that drives a protocol step serialize on it while keeping the per-list locks for producer AppendBuffer:

private readonly object m_sslGate = new object();

public void Write(byte[] pData, int dataLen)
{
    lock (m_sslGate)
    {
        AppendBuffer(m_pendingWriteList, new DataBuffer(pData, dataLen));
        RunSSL();
    }
}

public void ReadCompleted(byte[] pData, int dataLen)
{
    lock (m_sslGate)
    {
        AppendBuffer(m_pendingReadList, new DataBuffer(pData, dataLen));
        RunSSL();
    }
}

Because Monitor is re-entrant, same-thread nesting of RunSSL (a real possibility given the callback-driven data flow) is fine; the gate just prevents a second thread from entering the whole processing path. Keep AppendBuffer's internal list lock only for any producer that appends without driving RunSSL.

private bool m_fatalError;
private bool m_disposed;

Expand Down Expand Up @@ -277,8 +279,16 @@ protected void RunSSL()

while (!m_fatalError && ((!m_readRequired && dataToWrite) || dataToRead))
{
int pendingReadBefore = m_pendingReadList.Count;
int pendingWriteBefore = m_pendingWriteList.Count;
int pendingReadBefore;
int pendingWriteBefore;
lock (m_readListLock)
{
pendingReadBefore = m_pendingReadList.Count;
}
lock (m_writeListLock)
{
pendingWriteBefore = m_pendingWriteList.Count;
}
bool readRequiredBefore = m_readRequired;
var bioPendingBefore = Native.BIO_ctrl_pending(m_pBioOut);

Expand All @@ -304,8 +314,18 @@ protected void RunSSL()

GetPendingOperations(ref dataToRead, ref dataToWrite);
var bioPendingAfter = Native.BIO_ctrl_pending(m_pBioOut);
if (pendingReadBefore == m_pendingReadList.Count &&
pendingWriteBefore == m_pendingWriteList.Count &&
int pendingReadAfter;
int pendingWriteAfter;
lock (m_readListLock)
{
pendingReadAfter = m_pendingReadList.Count;
}
lock (m_writeListLock)
{
pendingWriteAfter = m_pendingWriteList.Count;
}
if (pendingReadBefore == pendingReadAfter &&
pendingWriteBefore == pendingWriteAfter &&
readRequiredBefore == m_readRequired &&
bioPendingBefore == bioPendingAfter)
{
Expand All @@ -323,26 +343,36 @@ protected void RunSSL()
}
}

public void Write(byte[] pData,int dataLen)
private readonly object m_sslGate = new object();

public void Write(byte[] pData, int dataLen)
{
DataBuffer pBuffer = new DataBuffer(pData, dataLen);
AppendBuffer(m_pendingWriteList, pBuffer);

RunSSL();
lock (m_sslGate)
{
AppendBuffer(m_pendingWriteList, new DataBuffer(pData, dataLen));
RunSSL();
}
}

public void ReadCompleted(byte[] pData, int dataLen)
{
DataBuffer pBuffer = new DataBuffer(pData, dataLen);
AppendBuffer(m_pendingReadList, pBuffer);

RunSSL();
lock (m_sslGate)
{
AppendBuffer(m_pendingReadList, new DataBuffer(pData, dataLen));
RunSSL();
}
}

private void GetPendingOperations(ref bool dataToRead, ref bool dataToWrite)
{
dataToRead = m_pendingReadList.Count > 0;
dataToWrite = m_pendingWriteList.Count > 0;
lock (m_readListLock)
{
dataToRead = m_pendingReadList.Count > 0;
}
lock (m_writeListLock)
{
dataToWrite = m_pendingWriteList.Count > 0;
}
}

private void PerformRead()
Expand Down Expand Up @@ -437,23 +467,35 @@ private void UseData(DataBufferList list, DataBuffer pBuffer, int result)

private void AppendBuffer(DataBufferList list, DataBuffer pBuffer)
{
list.AddLast(pBuffer);
object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock;
lock (lockObject)
{
list.AddLast(pBuffer);
}
}

DataBuffer GetNextBuffer(DataBufferList list)
{
DataBuffer head = null;
if (list.Count > 0)
object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock;
lock (lockObject)
{
head = list.First.Value;
list.RemoveFirst();
if (list.Count > 0)
{
head = list.First.Value;
list.RemoveFirst();
}
}
return head;
}

void PutbackBuffer(DataBufferList list, DataBuffer pBuffer)
{
list.AddFirst(pBuffer);
object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock;
lock (lockObject)
{
list.AddFirst(pBuffer);
}
}

/// <summary>
Expand Down