Calling "close" on MailClient which was only ever used for failed connection attempts does not call close on underlying NetClientImpl

[sfitts]

Version

Vert.x 3.9.0

Context

We discovered this when one of the servers in our production cluster ran out of memory. The heap dump showed a large number (around 10,000) of NetClientImpls being held in memory by only their associated closeHook. We eventually tracked this back to code which was creating a non-shared MailClient instance with invalid credentials for the target email account. That code would create the client, attempt to send an email message, fail, and call close on the client. However, we discovered that in this case the underlying NetClientImpl used to create the connection to the email server (which was successful) did not have close called on it. BTW - the 10K instances here are holding about 2GB of memory, mostly due to SSL handling.

Do you have a reproducer?

We have not yet built a full reproducer yet.

Steps to reproduce

Run code somewhat like this. The key is to make sure that the send attempt generates an error after the connection is established (so bad credentials work):

        MailConfig mailConfig = new MailConfig()
                .setHostname("mailHost")
                .setPort(465)
                .setSsl(true)
                .setUsername("emailUser")
                .setPassword("badPassword")
        MailClient mailClient = MailClient.create(Vertx.vertx(), mailConfig)
        MailMessage message= new MailMessage()
                .setFrom("emailAddress")
                .setSubject("Subject")
                .setText("body")
        mailClient.sendMail(message, { result ->
            mailClient.close() // This won't close the net client instance
        })

       // Wait here to see the close (which never occurs).  If the sending of mail succeeds then it will be called.

The expected result is that when the close is issued on the mailClient, the underlying netClient will also be closed -- but it never will be.

Extra

The reason for this is this is logic in SMTPConnectionPool.closeAllConnections. When we call that routine, if the connection count is 0 (which in this case it is due to the send failure) we skip all work. Unfortunately that means that we don't call close on the netClient property. That is only done in connectionClosed and then only when the pool as a whole has been closed (which it has not been when the failed connection is closed earlier). As a result the NetClientImpl is never closed and leaks. I think a simple addition of netClient.close() at line 217 of SMTPConnectionPool would fix this.