《8小时转职Golang工程师》超时强踢功能的BUG修复

最近在看刘丹冰的《8小时转职Golang工程师》

刘老师讲的思路很好，但有些细节没有处理好，很多人在跟着写代码后可能出现一些新手看来不太容易发现和解决的Bug。

还给大神发了邮件问：

邮件

刘丹冰Aceld大神

您好

我最近在看您的《8小时转职Golang工程师》，在学到第44集-超时强踢功能时发现了一个可能的潜在BUG。

当server.go中的Handler函数调用conn.Close()后，由于user.go中ListenMessage()函数仍在监听user的channel，最后的“您被踢了”消息可能在conn.Close() ”后发送，这会导致向已关闭的连接中写入消息报错。而您的代码中没有打印conn.Write([]byte(msg + “\n”))返回的报错信息，所以没有监测到这个问题。

我遇到了这个错误后很久才定位原因，在视频的评论区里也有很多人提出当接收到“您被踢了”消息后CPU占用率飙升。这可能是因为他们使用的也是Goland IDE，而Goland会提示在代码中添加捕获conn.Write([]byte(msg + “\n”))返回的报错信息，当直接打印时由于外层有for循环，会无限打印报错，导致cpu占用率飙升。

您可以在user.go中更改ListenMessage()方法复现这个问题：

func (this *User) ListenMessage() {
	for {
		msg := <-this.C

​		_, err := this.conn.Write([]byte(msg + "\n"))
​		if err != nil {
​			fmt.Println(err)
​		}
​	}
}

ListenMessage()这个协程应该如何正确关闭呢，希望得到您的指教！

分析问题

我遇到这个Bug后半天也没找到问题在哪，因为这可太巧了。

首先我打印bug是用println，当时写的时候用的tobnine自动补全这行代码，所以日志信息看起来就像系统报错（写的很正式，因为我是新手，半天没发现这行报错提示是我自己定义的）。其次这个报错是在for循环中，所以控制台会无限打印报错信息，然后导致cpu使用率飙升。最后我打印bug的时候用的是err而不是err.Error()，前者只能看见地址信息（刚开始不知道还有这区别）。

Golang把所有Excepiton都用Error来处理，我可太不习惯了。第一次都没想到这是一个报错，堆栈信息啥啥没有，我都不知道哪个文件第几行代码出错了。后来能换Panic换Panic…..

好了，先给出解决方法。我假设你已经看过刘丹冰《8小时转职Golang工程师》第44集，并且遇到了和我一样的问题。

这个问题出现的原因邮件里已经说清楚了，就是server.go中超时强踢时已经关闭了conn，但是user.go可能还会监听消息并向用户写入消息。

解决方案1

在打印报错后加一个return退出ListenMessage()协程，这样写虽然不会再出现无限打印，但属于出现错误后再处理。我认为最好的方式还是通过设计避免错误出现。如果出现错误后return可能会造成日志打印很多无效报错，或者如果不打印报错的话会丢失可能的报错提示。

解决方案2

我认为最佳解决方案是让这两个协程进行通信，让ListenMessage()知道conn已经关闭了。

这里用到一个特性，for range channel

• 场景：当需要不断从channel读取数据时
• 原理：使用for-range读取channel，这样既安全又便利，当channel关闭时，for循环会自动退出，无需主动监测channel是否关闭，可以防止读取已经关闭的channel，造成读到数据为通道所存储的数据类型的零值。
• 用法：

for x := range ch{
    fmt.Println(x)
}

我们可以这样更改，server.go中只关闭u.C，ListenMessage()检测到u.C关闭后不再写入信息。

user.go

func (u *User) ListenMessage() {
  //当u.C通道关闭后，不再进行监听并写入信息
   for msg := range u.C {
      _, err := u.conn.Write([]byte(msg + "\n"))
      if err != nil {
        panic(err)
      }
   }
  //不监听后关闭conn，conn在这里关闭最合适
   err := u.conn.Close()
   if err != nil {
      panic(err)
   }
​
}

server.go

func (s *Server) Handler(conn net.Conn) {
​
  user := NewUser(conn, s)
  user.Online()
​
  isLive := make(chan bool)
  go func() {
    buf := make([]byte, 4096)
    for {
      n, err := conn.Read(buf)
      //下线的时候会发长度为0的消息
      if n == 0 {
        user.Offline()
        //这样的写法会造成服务端出现大量CLOSE_WAIT，return也只是退出当前协程，而不是Handle
        return
      }
      //读到末尾的时候err是io.EOF
      if err != nil && err != io.EOF {
        fmt.Println("Error:", err.Error())
        return
      }
      //去除最后的'\n'并转为字符串
      msg := string(buf[:n-1])
      user.DoMessage(msg)
      isLive <- true
    }
​
  }()
  // 空select会一直阻塞
  for {
    //超时强踢
    select {
    case <-isLive:
    //当前用户是活跃的,不做任何事情，select中会执行下面这句重置
    //time.After(time.Second * 10)重新执行即刻重置定时器，定时到后会发送信息
    //这里select 第二个case定时器触发后，处于阻塞状态。当满足第一个 case 的条件后，
    //打破了 select 的阻塞状态，每个条件又开始判断，第2个 case 的判断条件一执行，就重置定时器了。
    case <-time.After(time.Second * 10):
      user.SendMessage("超过10秒未操作")
      //只关闭uer.C
      close(user.C)
      return
    }
  }
​
}
​

这样问题就解决了！！！