Skip to content
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.

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

SocketByteHandler is leaked when upgrading to SSL #1247

Closed
bgrainger opened this issue Dec 6, 2022 · 5 comments
Closed

SocketByteHandler is leaked when upgrading to SSL #1247

bgrainger opened this issue Dec 6, 2022 · 5 comments
Assignees
Labels

Comments

@bgrainger
Copy link
Member

When ServerSession connects to a server via TCP, a SocketByteHandler is created:

var byteHandler = m_socket is null ? new StreamByteHandler(m_stream!) : (IByteHandler) new SocketByteHandler(m_socket);

If the connection is upgraded to SSL, InitSslAsync replaces the ByteHandler with a StreamByteHandler:

var sslByteHandler = new StreamByteHandler(sslStream);
m_payloadHandler!.ByteHandler = sslByteHandler;

At this point, the original SocketByteHandler is leaked, along with the TCP socket, the SocketAwaitable, an OverlappedData, etc.

@bgrainger bgrainger added the bug label Dec 6, 2022
@bgrainger bgrainger self-assigned this Dec 6, 2022
@bgrainger
Copy link
Member Author

Because the SocketAsyncEventArgs is never disposed, the pinned GC Handle it creates is never unpinned, which keeps an object graph alive in memory forever (it's not cleaned up by finalization).

@sanderwollaert
Copy link

Hi @bgrainger,
We are using 0.69.9 in our .net framwork/.net core application and upgrading all the way to at least v2.2.2, in our codebase will be quite hard. Is there an option to merge the commit to the 0.x branch and get a new release for 0.x?
I'm willing to contribute to the work needed.
Thanks in advance

@bgrainger
Copy link
Member Author

The "v0.x" branch is still present (https://github.com/mysql-net/MySqlConnector/tree/v0.x). Since it's a two-line fix, it probably won't be too difficult to port back to https://github.com/mysql-net/MySqlConnector/blob/v0.x/src/MySqlConnector/Protocol/Serialization/StandardPayloadHandler.cs#L34-L38.

However, I highly doubt that any of the CI infrastructure set up for it still works, so it may be very difficult to publish a new version.

I would strongly recommend you make the fix locally then "publish" your own private version of MySqlConnector, e.g., version 0.69.11-yourcompany.1 to an internal repo. This will be greater than the public 0.69.10 so it will be seen as an upgraded version, but it will be less than 0.69.11 if that were ever officially released, and the yourcompany version suffix will make sure there's no conflict. (And you can release -yourcompany.2, etc. if you ever make future changes.)

@bgrainger
Copy link
Member Author

However, I highly doubt that any of the CI infrastructure set up for it still works, so it may be very difficult to publish a new version.

Just to be clear, because I think this will be a lot of effort, I do not plan to accept a contribution for this fix to the old branch and try to publish a new 0.69.11 release from the old code. You should plan to produce and use your own local private version with the fix.

@sanderwollaert
Copy link

Thanks for the quick answer. We will make a private build for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants