tgulacsi / goracle

cx_Oracle translation for Oracle access
Other
47 stars 16 forks source link

ExternalLobVar not thread safe #26

Closed qqiao closed 9 years ago

qqiao commented 9 years ago

Minimum program to reproduce:

text := "abcdefghijkl"
stmt, err := conn.Prepare("SELECT TO_CLOB('" + text + "') FROM DUAL")
if err != nil {
    log.Printf("error preparing query1: %v", err)
}
defer stmt.Close()

for i := 0; i < 50; i++ {
    go func() {
        for true {
            var clob *oracle.ExternalLobVar
            if err = stmt.QueryRow().Scan(&clob); err != nil {
                log.Printf("Error scanning clob: %v", err)
            }
            defer clob.Close()
            log.Printf("clob: %v", clob)

            got, err := clob.ReadAll()
            if err != nil {
                log.Printf("error reading clob: %v", err)
            }
            if string(got) != text {
                log.Printf("clob: got %q, awaited %q", got, text)
            }
        }
    }()
}

<-make(chan int)

Output:

2015/01/26 17:13:41 error reading clob: cannot get internal size of <ExternalLobVar of <Variable ClobVar of 0x0>>: -1073741817: invalid handle
2015/01/26 17:13:41 clob: got "", awaited "abcdefghijkl"
2015/01/26 17:13:41 clob: <ExternalLobVar of <Variable ClobVar of 0x0>>
2015/01/26 17:13:41 CheckStatus(-2) @LobGetLength ERR=-1073741817: invalid handle
qqiao commented 9 years ago

Just to add more info: according to Go documentation, stmt is supposed to be safe to be used across multiple goroutines, but I've decided to put it inside the goroutine just to eliminate that uncertainty:

text := "abcdefghijkl"

for i := 0; i < 50; i++ {
    go func() {
        stmt, err := conn.Prepare("SELECT TO_CLOB('" + text + "') FROM DUAL")
        if err != nil {
            log.Printf("error preparing query1: %v", err)
        }
        defer stmt.Close()

        for true {
            var clob *oracle.ExternalLobVar
            if err = stmt.QueryRow().Scan(&clob); err != nil {
                log.Printf("Error scanning clob: %v", err)
            }
            defer clob.Close()
            log.Printf("clob: %v", clob)

            got, err := clob.ReadAll()
            if err != nil {
                log.Printf("error reading clob: %v", err)
            }
            if string(got) != text {
                log.Printf("clob: got %q, awaited %q", got, text)
            }
        }
    }()
}

<-make(chan int)

And it exhibits the same issue.

tgulacsi commented 9 years ago

It seems that this is not really with ExternalLobVar but with stmt. But anyway you cannot use the same statement concurrently - each QueryRow will run a new query on the same cursor, invalidating the previous cursor, and all the handles (lobvar) associated with it.

BUT there is something fishy somewhere: see TestGetLobConcurrent: that prepares a different statement for each goroutine, but suffers from the same error! I'll have to dig into it more deeply...

qqiao commented 9 years ago

I constructed the original test cases according to the go documentation for DB.Prepare

Prepare creates a prepared statement for later queries or executions. Multiple queries or executions may be run concurrently from the returned statement.

I hope I'm not mis-interpreting the documentation though.

Also, in my 2nd comment, I also tried creating a new stmt for each goroutine, and it still fails with the same error.

tgulacsi commented 9 years ago

Hi,

Check out the documentation of driver Prepare: http://golang.org/pkg/database/sql/driver/#Conn and golang.org/pkg/database/sql/driver/#Stmt: "Stmt is a prepared statement. It is bound to a Conn and not used by multiple goroutines concurrently. "

So database/sql does the magic here, and I need to check out what. Till that: using directly (goracle/oracle/lob_test.go) works as intended.

tgulacsi commented 9 years ago

As database/sql closes the connection under the lob, use SetMaxOpenConns and SetMaxIdleConns to avoid that!

tgulacsi commented 9 years ago

Ok, I've found the real issue: QueryRow assumes there will be only one row, and closes the uderlying Connection right after Scan. So the fix is simple: don't be lazy, and use Query, and the returned Rows if you need to use the database connection after Scan.