记一次因为结构体复用导致的BUG

y1nhui · · 732 次点击 · · 开始浏览    
这是一个创建于 的文章,其中的信息可能已经有所发展或是发生改变。

# 结构体复用BUG 本文首发作者个人博客:https://y1nhui.com/ ## 前文 先看一行代码 `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 函数,看到的第一个函数是 ``` go 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不会有缓冲区的问题,是因为它在函数内单独定义了一个临时的缓冲区 ```go 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在过去曾经被提及,但是最后并未被处理的原因了。 比如我们使用一个结构体 ``` go type Account struct { Name string Test } type Test struct { Test string } ``` 这里的 test 它并不是必传的。 测试在第一次传递了一个错误的test进来,导致了报错。同时在我们的内存中,s 对应的 Account{}被永远赋予了一个错误的test的值。 直到某次传入参数中,附带了一个正确的test,将它纠正。 而如果测试只是传递一个错误的 Name ,在下一次赋值又会将错误的Name给覆盖了,然后一切正常。 ## 修复方法 ### 临时变量 (未验证) 最开始的想法是 手动创建一个 `tmpS:=s` 不就ok了 ``` go 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 的方法 ```go 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 一个新的结构体 这里参考了一个第三方库的写法,可惜的是在写本博客的时候已经有些想不起来是哪个库了 ```go 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()`函数,如果要优化的话,应该怎么做

有疑问加站长微信联系(非本文作者))

入群交流(和以上内容无关):加入Go大咖交流群,或添加微信:liuxiaoyan-s 备注:入群;或加QQ群:692541889

732 次点击  
加入收藏 微博
暂无回复
添加一条新回复 (您需要 登录 后才能回复 没有账号 ?)
  • 请尽量让自己的回复能够对别人有帮助
  • 支持 Markdown 格式, **粗体**、~~删除线~~、`单行代码`
  • 支持 @ 本站用户;支持表情(输入 : 提示),见 Emoji cheat sheet
  • 图片支持拖拽、截图粘贴等方式上传