Avoid unclosed sockets while opening connections

[oserde]

When connections could not be properly established (e.g. wrong password, database in maintenance, etc.) they were not being closed. This might be dangerous when making lots of connections, as the number of sockets (and hence the number of open files) pile up.

[MichaelS11]

@oserde I believe the Go SQL code handles this. Do you have test code showing otherwise?

[oserde]

Hi @MichaelS11 . Thank you for your quick response.

I have been using this code for debugging:

package main

import (
    "database/sql"
    "fmt"
    "github.com/mattn/go-oci8"
    "log"
    "os"
    "time"
)

func main() {

    oci8.OCI8Driver.Logger = log.New(os.Stderr, "oci8 ", log.Ldate|log.Ltime|log.LUTC|log.Llongfile)

    db, err := sql.Open("oci8", "myuser/mypass@mydb:1527/xxx")
    if err != nil {
        fmt.Printf("Open error is not nil: %v", err)
        return
    }
    if db == nil {
        fmt.Println("db is nil")
        return
    }

    err = db.Ping()
    if err != nil {
        fmt.Printf("Error: %v", err)
        time.Sleep(time.Minute * 10)
        return
    }
}

Using a wrong password makes the Ping() call to throw an error. I put the Sleep() call to be able to check open sockets by the process. I have also checked tcp connections. The connection is not closed. In fact, it remains as ESTABLISHED. Eventually the server sends an RST packet due to inactivity. But the socket remains there.

I tried to use db.Close(), but it is useless in this case, because when it gets to this line , variable db.freeConn is empty, and no sockets are closed.

So this PR intends to fix two problems:

1. Sockets might not be closed after a failed connection.
2. Even though we wanted to fix this behaviour externally and call db.close() when Ping() throws an error, this call to db.Close() is unable to close the sockets, because the connection is not inside db.freeConn.

Do you think this behaviour should be implemented by go database/sql library?

Thank you.

[MichaelS11]

See the issue, working on it.

[MichaelS11]

Should be fixed in https://github.com/mattn/go-oci8/pull/339 Please test

[MichaelS11]

@oserde Please close this PR if https://github.com/mattn/go-oci8/pull/339 fix the issue.

[oserde]

Hi @MichaelS11 . PR #339 definitely fixes the issue. Thank you!

Closing in favor of #339