MySQL Bugs: #82810: TcpClient missing implementations

Bug #82810	TcpClient missing implementations
Submitted:	31 Aug 2016 7:38	Modified:	20 Jul 2020 19:06
Reporter:	Michiel Hazelhof	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector / NET	Severity:	S2 (Serious)
Version:	7.0.4 dmr	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Connection, keepalive, SuppressFinalize, TcpClient

Description:
The new GetTcpStream implementation uses TcpClient instead of the old socket implementation.

While on the larger scale this is no big deal (it's just a wrapper and probably a bit more NetCore compatible), there are a few things that have gone missing.

1: As noted by the orriginal comment keepalive is no longer implemented
2: SuppressFinalize is no longer called, resulting in "(Got an error reading communication packets)" server side when closing a winforms application and the destructor calls .Close() on the connection (there is a chance that the garbage collection has beaten the destructor)

How to repeat:
0: Monitor the servers warning log
1: Create a WinForms application
2: Create a simple connector wrapper
3: Use the destructor in the wrapper to close the connection
4: Open a connection using the wrapper
5: Close the application using the close button

Suggested fix:
StreamCreator.cs:

        private static Stream GetTcpStream(MySqlConnectionStringBuilder settings)
        {
            TcpClient client = new TcpClient(AddressFamily.InterNetwork);
            Task task = client.ConnectAsync(settings.Server, (int)settings.Port);

            if (!task.Wait((int)settings.ConnectionTimeout*1000))
                throw new MySqlException(Resources.Timeout);

            // Implement KeepAlive
            MyNetworkStream.SetKeepAlive(client.Client, settings.Keepalive);
            NetworkStream result = client.GetStream();

            // Suppres finalization, else external destructors might be unable to close the connection in a normal fashion resulting in "(Got an error reading communication packets)" on the server side
            GC.SuppressFinalize(result);

            return result;
        }

Hello Michiel Hazelhof,

Thank you for the bug report.
Could you please provide repeatable test case (sample project, wrapper class, connection destructor etc. - please make it as private if you prefer) to confirm this issue at our end?

Thanks,
Chiranjeevi.

Please take a look at your own source code "MySql.Data\common\StreamCreator.cs:90": "//TODO:  reimplement or remove keepalive".

The usage of TcpClient means there is a convenient wrapper, but it does not deliver keepalive's etc out of the box.

I'm not going to deliver test cases as this should be painfully obvious to anyone with even basic network and coding skills (it's even mentioned in the official source code!).

Hello Michiel,

Thank you for the feedback.
Verified based on internal discussion with dev's.

Thanks,
Chiranjeevi.

Posted by developer:
 
Fixed as of the upcoming MySQL Connector/NET 8.0.22 release, and here's the proposed changelog entry from the documentation team:

The TcpClient implementation limited some of the connection options when
an external wrapper from a Windows Forms application made the connection.
This fix enables the related external destructor to be called without
returning an error.

Thank you for the bug report.