一次结构体复用导致的bug

结构体复用BUG

前文

先看一行代码 ws.Route(ws.Get("xxx")).To(handler.xx).Filter(handler.BodyFilter(&param.Crate{}))

咋一看似乎没用问题，为该路由增加校验规则，校验的类型是 param.Crate{}。

至于 BodyFilter 函数内部是什么样的，先不谈。先谈本次遇到的问题

环境，BUG与定位

环境

环境是运行在k8s集群内的一个webserver pod。

集群内的流量管理是基于 istio做的。

BUG表现

测试对环境做破坏性测试的时候，为Create API 传递了一些不合法的值，成功被过滤拦截，并且返回报错。

但是之后测试发现了一个问题，正常的Create请求也被拦截，一并返回报错，并且报错内容与之前的报错一致。

定位

这种问题，很明显就是哪里有了缓存。就是不知道在哪里有了缓存。

排除istio缓存

测试环境的 webserver 是双副本在跑，笔者直接打开两副本的log，传递正确报文。

发现 podA 与 podB 方便给出了不同反应。

当流量打到 podA 上后，返回了成功报文，对应的创建操作正常运行。而流量打到 podB 后，则会返回测试反馈的现象：错误

istio的 virtualService 配置 host 是一个svc，而非具体的 pod名，既然可以稳定只在 podB 复现问题，基本可以排除是istio层面缓存了请求的可能。因而在这里假定缓存问题出在了pod上面，也就是程序本身

排除报文缓存

接着笔者就在猜想，是不是报文缓存。

如读取报文内容使用的是拷贝读取，但是对应的字节仍然在缓冲区中，正确返回时应该会清空缓存区，但是错误返回跳过了清空这一步。于是触发下一次读取时，继续从缓冲区中读到了错误内容。

于是笔者跟进BodyFilter 函数，看到的第一个函数是

func BodyFilter(s interface{}) restful.FilterFunction {
	return func(req *restful.Request, resp *restful.Response, chain *restful.FilterChain) {
     ...
  	if bys, err = io.ReadAll(req.Request.Body); err != nil {
			return
		}
    	if err = json.Unmarshal(bys, tmpS); err != nil {
			return
		}
    ...
	}
}

坏消息，和缓冲区无关，比较 io.ReadAll函数是定义在函数内的一个临时缓冲区，当函数返回时就应该等待被GC了，不会被复用

好消息是我看到问题了，这里用了一个闭包

定位

问题一下清晰了，就如同上文提到了，io.ReadAll不会有缓冲区的问题，是因为它在函数内单独定义了一个临时的缓冲区

func ReadAll(r Reader) ([]byte, error) {
	b := make([]byte, 0, 512)
	for {
		n, err := r.Read(b[len(b):cap(b)])
		b = b[:len(b)+n]
		if err != nil {
			if err == EOF {
				err = nil
			}
			return b, err
		}

		if len(b) == cap(b) {
			// Add more capacity (let append pick how much).
			b = append(b, 0)[:len(b)]
		}
	}
}

这里的 b := make([]byte, 0, 512),它本身并没有被返回，所以当函数执行完成后便会在GC等待被标记，之后回收

但是我们的BodyFilter{} 函数，这里却用了一个闭包，代表着本来期望着的临时变量 s interface 永远在那里。它会被不断赋值，但是不被置空。

这样也就解释了，为什么这个bug在过去曾经被提及，但是最后并未被处理的原因了。

比如我们使用一个结构体

type Account struct {
Name string
Test
}

type Test struct {
  Test string
}

这里的 test 它并不是必传的。测试在第一次传递了一个错误的test进来，导致了报错。同时在我们的内存中，s 对应的 Account{}被永远赋予了一个错误的test的值。

直到某次传入参数中，附带了一个正确的test，将它纠正。

而如果测试只是传递一个错误的 Name ，在下一次赋值又会将错误的Name给覆盖了，然后一切正常。

修复方法

临时变量 (未验证)

最开始的想法是手动创建一个 tmpS:=s 不就ok了

func BodyFilter(s interface{}) restful.FilterFunction {
  tmpS:=s
	return func(req *restful.Request, resp *restful.Response, chain *restful.FilterChain) {
     ...
  	if bys, err = io.ReadAll(req.Request.Body); err != nil {
			return
		}
    	if err = json.Unmarshal(bys, tmpS); err != nil {
			return
		}
    ...
	}
}

但是这一步被同事制止了，表示这里是否有可能获取到一个nil的指针对象即 tmpS = &Account{} -> nil

笔者还未来得及验证，事后如果有验证将会补充到评论区

函数参数

这里我们的传入其实主要是为了绑定body 与它的对应的 validator 的条件

那么我们完全可以将s 的类型变为一个会返回interface 的方法

func RequestBodyParamFilterTest(s func() interface{}) restful.FilterFunction {
	return func(req *restful.Request, resp *restful.Response, chain *restful.FilterChain) {
		tmpS := s()
    ...
	}
}

ws.Route(ws.Get("xxx")).To(handler.xx).Filter(handler.BodyFilter(func() interface{} {
		return &Create{}
	}))

这样，简洁方便。唯一的问题就是 handler.BodyFilter 函数调用的地方太多，设计多个仓库，改动略大

通过反射创建

还有一种方法就是基于反射在函数内 new 一个新的结构体

这里参考了一个第三方库的写法，可惜的是在写本博客的时候已经有些想不起来是哪个库了

func alloc(s interface{}) interface{} {
	vv := reflect.ValueOf(s)
	if vv.Kind() == reflect.Ptr {
		return reflect.New(vv.Elem().Type()).Interface()
	}
	return reflect.New(vv.Type()).Interface()
}

这个方法看起来有些难以理解，但是改动最小，在做了一些单测验证可用后便作为改动方法合入了代码中

结语

本文总结了前段时间遇到了一个因为结构体复用在校验方法中的bug复现，定位于修复的方法。

虽然最后写出来发现这个困扰了一个下午的问题，其实根因倒是蛮简单的。

很多bug的解决反而是简单的，最大的难点在于如何定位。

结尾预留给笔者自己和读者两个点的问题

一个是上文的直接等于，是否可能出现指向nil的新变量

另一个是，此处或许可以用泛型的方法优化BodyFilter()函数，如果要优化的话，应该怎么做