全部连接被关闭后导致hang（阻塞）

[upwell]

现象：

当有较多的goroutine在等待从Pool中Get连接，同时没有空闲连接时，有种情况，当被使用的连接，同时出错并被调用者Close掉时，等待获取连接的goroutine会一直阻塞等待从connReq这个channel获取连接，而所有的连接都已经被关闭了，不会再有可用的连接，导致一直等待，程序hang住。

原因

在Get方法中，第133行检查完已创建的连接数达到了最大值后，会等待从connReq的channel等待其它goroutine Put回来的连接。
在Close方法中，会把openingConns--，但已经没法触发Get方法去新建连接了，Get方法都被阻塞在从connReq的channel拿连接了。

Get方法133行

pool/channel.go

Line 133 in 58e025e

if c.openingConns >= c.maxActive {

pool/channel.go

Lines 130 to 140 in 58e025e

	default:
	c.mu.Lock()
	log.Debugf("openConn %v %v", c.openingConns, c.maxActive)
	if c.openingConns >= c.maxActive {
	req := make(chan connReq, 1)
	c.connReqs = append(c.connReqs, req)
	c.mu.Unlock()
	ret, ok := <-req
	if !ok {
	return nil, ErrMaxActiveConnReached
	}

Close方法

pool/channel.go

Lines 202 to 213 in 58e025e

	func (c *channelPool) Close(conn interface{}) error {
	if conn == nil {
	return errors.New("connection is nil. rejecting")
	}
	c.mu.Lock()
	defer c.mu.Unlock()
	if c.close == nil {
	return nil
	}
	c.openingConns--
	return c.close(conn)
	}

[silenceper]

多谢反馈。 这里的问题应该是原本Get的请求会被阻塞住而导致一直无法获得连接，而如果是新建的Get方法，还是会通过新建连接返回。

这里应该在这里等待加一个超时返回，你觉得呢？ @upwell

[upwell]

@silenceper
之前有考虑解法，有两个思路：

• 一个是超时重试
• 一个是在Close的时候往connReq发消息，让Get方法重新循环

超时重试可以解决后续可能出现的类似问题，但超时时间要斟酌一下，设的太短，容易误触发重试，设的太长，可能会影响业务逻辑。

我用了解法2改了个版本，相对简单些，就只是解决这个特定问题了。

Close方法

type connReq struct {
	idleConn *idleConn
	reCheck  bool
}

func (c *channelPool) Close(conn interface{}) error {
	if conn == nil {
		return errors.New("connection is nil. rejecting")
	}
	c.mu.Lock()
	defer c.mu.Unlock()
	if c.close == nil {
		return nil
	}
	c.openingConns--

	if l := len(c.connReqs); l > 0 {
		req := c.connReqs[0]
		copy(c.connReqs, c.connReqs[1:])
		c.connReqs = c.connReqs[:l-1]
		req <- connReq{
			idleConn: nil,
			reCheck:  true,
		}
	}

	return c.close(conn)
}

Get方法

  if c.openingConns >= c.maxActive {                                                                                                                                                                              
      req := make(chan connReq, 1)                                                                    
      c.connReqs = append(c.connReqs, req)                                                            
      c.mu.Unlock()                                                                                   
      ret, ok := <-req                                                                                
      if !ok {                                                                                        
          return nil, ErrMaxActiveConnReached                                                         
      }                                                                                               
      if ret.reCheck {                                                                                
          continue                                                                                    
      }                                                                                               
                                                                                                      
      if timeout := c.idleTimeout; timeout > 0 {                                                      
          if ret.idleConn.t.Add(timeout).Before(time.Now()) {                                         
              //丢弃并关闭该连接                                                                      
              c.Close(ret.idleConn.conn)                                                              
              continue                                                                                
          }                                                                                           
      }                                                                                               
      return ret.idleConn.conn, nil                                                                   
  }    

[silenceper]

看了下database/sql中提供的连接池，通过使用者传入context包，让调用方进行控制

[upwell]

@silenceper 昨天仔细追了下database/sql的代码，发现其实是在finalClose方法里面做了判断，对于需要关闭的连接，根据情况判断是否需要开一个新的连接，这个思路挺好的，可以借鉴过来。

	dc.db.maybeOpenNewConnections()

func (dc *driverConn) finalClose() error {
	var err error

	// Each *driverStmt has a lock to the dc. Copy the list out of the dc
	// before calling close on each stmt.
	var openStmt []*driverStmt
	withLock(dc, func() {
		openStmt = make([]*driverStmt, 0, len(dc.openStmt))
		for ds := range dc.openStmt {
			openStmt = append(openStmt, ds)
		}
		dc.openStmt = nil
	})
	for _, ds := range openStmt {
		ds.Close()
	}
	withLock(dc, func() {
		dc.finalClosed = true
		err = dc.ci.Close()
		dc.ci = nil
	})

	dc.db.mu.Lock()
	dc.db.numOpen--
	dc.db.maybeOpenNewConnections()
	dc.db.mu.Unlock()

	atomic.AddUint64(&dc.db.numClosed, 1)
	return err
}

https://github.com/golang/go/blob/master/src/database/sql/sql.go#L574-L603

[silenceper]

这个可以用，我找时间把这块优化下

[ansoda]

我也遇到了同样的close后hang住的问题，看能否更新一个版本优化下

[upwell]

提了一个 pull request来修复这个问题，参考了 database/sql 的代码。 @silenceper 我放在自己的项目里面跑了一段时间，暂时没有发现问题。

[bli22ard]

提了一个 pull request来修复这个问题，参考了 database/sql 的代码。 @silenceper 我放在自己的项目里面跑了一段时间，暂时没有发现问题。

我看了下你修复方法， 大概思路是，在关闭的时候创建连接，用来激活 ret, ok := <-req ， 这种方式还会存在一些问题，如果createConnection一直返回失败，阻塞会一直进行下去，实际业务中， 你可能需要给前端返回系统错误，而不是后端一直处于等待状态，这种可能出现的无限等待不是很合理。 我觉得根本问题是获取连接没有超时时间，如果有超时时间，这些问题都迎刃而解

[Zenghao-CQ]

提了一个 pull request来修复这个问题，参考了 database/sql 的代码。 @silenceper 我放在自己的项目里面跑了一段时间，暂时没有发现问题。

我看了下你修复方法， 大概思路是，在关闭的时候创建连接，用来激活 ret, ok := <-req ， 这种方式还会存在一些问题，如果createConnection一直返回失败，阻塞会一直进行下去，实际业务中， 你可能需要给前端返回系统错误，而不是后端一直处于等待状态，这种可能出现的无限等待不是很合理。 我觉得根本问题是获取连接没有超时时间，如果有超时时间，这些问题都迎刃而解

确实如此，这个从fatih/poolfork的项目给Get方法加了ctx，感觉稳定很多。