Merge pull request go-sql-driver#383 from mrsinham/thrdsafeinfile

Race on LOAD DATA called from different routines

Author: arnehormann

AUTHORS

@@ -37,6 +37,7 @@ Soroush Pour <me at soroushjp.com>
 Stan Putrya <root.vagner at gmail.com>
 Xiaobing Jiang <s7v7nislands at gmail.com>
 Xiuming Chen <cc at cxm.cc>
+Julien Lefevre <julien.lefevr at gmail.com>
 
 # Organizations
 

README.md

@@ -331,7 +331,7 @@ import "github.com/go-sql-driver/mysql"
 
 Files must be whitelisted by registering them with `mysql.RegisterLocalFile(filepath)` (recommended) or the Whitelist check must be deactivated by using the DSN parameter `allowAllFiles=true` ([*Might be insecure!*](http://dev.mysql.com/doc/refman/5.7/en/load-data-local.html)).
 
-To use a `io.Reader` a handler function must be registered with `mysql.RegisterReaderHandler(name, handler)` which returns a `io.Reader` or `io.ReadCloser`. The Reader is available with the filepath `Reader::<name>` then.
+To use a `io.Reader` a handler function must be registered with `mysql.RegisterReaderHandler(name, handler)` which returns a `io.Reader` or `io.ReadCloser`. The Reader is available with the filepath `Reader::<name>` then. Choose different names for different handlers and `DeregisterReaderHandler` when you don't need it anymore.
 
 See the [godoc of Go-MySQL-Driver](http://godoc.org/github.com/go-sql-driver/mysql "golang mysql driver documentation") for details.
 

infile.go

@@ -13,11 +13,14 @@ import (
 	"io"
 	"os"
 	"strings"
+	"sync"
 )
 
 var (
-	fileRegister   map[string]bool
-	readerRegister map[string]func() io.Reader
+	fileRegister       map[string]bool
+	fileRegisterLock   sync.RWMutex
+	readerRegister     map[string]func() io.Reader
+	readerRegisterLock sync.RWMutex
 )
 
 // RegisterLocalFile adds the given file to the file whitelist,
@@ -32,17 +35,21 @@ var (
 //  ...
 //
 func RegisterLocalFile(filePath string) {
+	fileRegisterLock.Lock()
 	// lazy map init
 	if fileRegister == nil {
 		fileRegister = make(map[string]bool)
 	}
 
 	fileRegister[strings.Trim(filePath, `"`)] = true
+	fileRegisterLock.Unlock()
 }
 
 // DeregisterLocalFile removes the given filepath from the whitelist.
 func DeregisterLocalFile(filePath string) {
+	fileRegisterLock.Lock()
 	delete(fileRegister, strings.Trim(filePath, `"`))
+	fileRegisterLock.Unlock()
 }
 
 // RegisterReaderHandler registers a handler function which is used
@@ -61,18 +68,22 @@ func DeregisterLocalFile(filePath string) {
 //  ...
 //
 func RegisterReaderHandler(name string, handler func() io.Reader) {
+	readerRegisterLock.Lock()
 	// lazy map init
 	if readerRegister == nil {
 		readerRegister = make(map[string]func() io.Reader)
 	}
 
 	readerRegister[name] = handler
+	readerRegisterLock.Unlock()
 }
 
 // DeregisterReaderHandler removes the ReaderHandler function with
 // the given name from the registry.
 func DeregisterReaderHandler(name string) {
+	readerRegisterLock.Lock()
 	delete(readerRegister, name)
+	readerRegisterLock.Unlock()
 }
 
 func deferredClose(err *error, closer io.Closer) {
@@ -90,7 +101,11 @@ func (mc *mysqlConn) handleInFileRequest(name string) (err error) {
 		// The server might return an an absolute path. See issue #355.
 		name = name[idx+8:]
 
-		if handler, inMap := readerRegister[name]; inMap {
+		readerRegisterLock.RLock()
+		handler, inMap := readerRegister[name]
+		readerRegisterLock.RUnlock()
+
+		if inMap {
 			rdr = handler()
 			if rdr != nil {
 				data = make([]byte, 4+mc.maxWriteSize)
@@ -106,7 +121,10 @@ func (mc *mysqlConn) handleInFileRequest(name string) (err error) {
 		}
 	} else { // File
 		name = strings.Trim(name, `"`)
-		if mc.cfg.allowAllFiles || fileRegister[name] {
+		fileRegisterLock.RLock()
+		fr := fileRegister[name]
+		fileRegisterLock.RUnlock()
+		if mc.cfg.allowAllFiles || fr {
 			var file *os.File
 			var fi os.FileInfo