Skip to content

Commit

Permalink
Merge pull request #1035 from bjornouderoelink/fix-concurrent-map-writes
Browse files Browse the repository at this point in the history
Fix #1031: concurrent map writes
  • Loading branch information
maddyblue authored Apr 21, 2021
2 parents b2901c7 + 69b14f1 commit 7f02e6b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 12 deletions.
10 changes: 8 additions & 2 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,13 @@ func (c *Connector) open(ctx context.Context) (cn *conn, err error) {
// the user.
defer errRecoverNoErrBadConn(&err)

o := c.opts
// Create a new values map (copy). This makes it so maps in different
// connections do not reference the same underlying data structure, so it
// is safe for multiple connections to concurrently write to their opts.
o := make(values)
for k, v := range c.opts {
o[k] = v
}

bad := &atomic.Value{}
bad.Store(false)
Expand Down Expand Up @@ -1100,7 +1106,7 @@ func isDriverSetting(key string) bool {
return true
case "password":
return true
case "sslmode", "sslcert", "sslkey", "sslrootcert":
case "sslmode", "sslcert", "sslkey", "sslrootcert", "sslinline":
return true
case "fallback_application_name":
return true
Expand Down
13 changes: 11 additions & 2 deletions conn_go18.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,16 @@ func (cn *conn) watchCancel(ctx context.Context) func() {
}

func (cn *conn) cancel(ctx context.Context) error {
c, err := dial(ctx, cn.dialer, cn.opts)
// Create a new values map (copy). This makes sure the connection created
// in this method cannot write to the same underlying data, which could
// cause a concurrent map write panic. This is necessary because cancel
// is called from a goroutine in watchCancel.
o := make(values)
for k, v := range cn.opts {
o[k] = v
}

c, err := dial(ctx, cn.dialer, o)
if err != nil {
return err
}
Expand All @@ -142,7 +151,7 @@ func (cn *conn) cancel(ctx context.Context) error {
c: c,
bad: bad,
}
err = can.ssl(cn.opts)
err = can.ssl(o)
if err != nil {
return err
}
Expand Down
8 changes: 0 additions & 8 deletions ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ func ssl(o values) (func(net.Conn) (net.Conn, error), error) {
return nil, err
}

// This pseudo-parameter is not recognized by the PostgreSQL server, so let's delete it after use.
delete(o, "sslinline")

// Accept renegotiation requests initiated by the backend.
//
// Renegotiation was deprecated then removed from PostgreSQL 9.5, but
Expand Down Expand Up @@ -89,9 +86,6 @@ func sslClientCertificates(tlsConf *tls.Config, o values) error {
sslinline := o["sslinline"]
if sslinline == "true" {
cert, err := tls.X509KeyPair([]byte(o["sslcert"]), []byte(o["sslkey"]))
// Clear out these params, in case they were to be sent to the PostgreSQL server by mistake
o["sslcert"] = ""
o["sslkey"] = ""
if err != nil {
return err
}
Expand Down Expand Up @@ -157,8 +151,6 @@ func sslCertificateAuthority(tlsConf *tls.Config, o values) error {

var cert []byte
if sslinline == "true" {
// // Clear out this param, in case it were to be sent to the PostgreSQL server by mistake
o["sslrootcert"] = ""
cert = []byte(sslrootcert)
} else {
var err error
Expand Down

0 comments on commit 7f02e6b

Please sign in to comment.