记一次ThreadLocal导致的生产事故
1.背景
前段时间发生了一起偶现生产事故,客服查询用户的订单列表时发现多出来了很多不应对其展示的订单。后经排查是由于ThreadLocal使用不当造成的。
2.过程
改动前
大致伪代码如下:
ThreadLocal<Boolean> IS_HIGHVENDER = ThreadLocal.withInitial(() -> false);Set<String> v_set = new HashSet<>();// set里面有一些指定的 vString v = "xx";//客服自身的某属性if(v_set.contains(v)){IS_HIGHVENDER.set(true);}try {List orderList = new ArrayList(); // 查出来的订单列表boolean other = false;// 其他过滤条件if(!IS_HIGHVENDER.get() || other ){orderList = new ArrayList(); // 对订单列表有一些过滤逻辑}}finally {IS_HIGHVENDER.remove();}return orderList;
改动后
大致伪代码:
ThreadLocal<Boolean> IS_HIGHVENDER = ThreadLocal.withInitial(() -> false);Set<String> v_set = new HashSet<>();// set里面有一些指定的 vString v = "xx";//客服自身的某属性if(v_set.contains(v)){IS_HIGHVENDER.set(true);}try {Callable callable1 = new Callable() {@Overridepublic Object call() throws Exception {try {List orderList = new ArrayList(); // 查出来的订单列表boolean other = false;// 其他过滤条件if (!IS_HIGHVENDER.get() || other) {orderList = new ArrayList(); // 对订单列表有一些过滤逻辑}return orderList;}finally {IS_HIGHVENDER.remove();}}};Callable callable2 = new Callable() {@Overridepublic Object call() throws Exception {// 一些其他业务return null;}};// 把两个任务丢线程池去执行了// 得到两个任务的结果后进行一些业务处理,return orderList; // 返回任务1中的订单列表}
分析
改动前是直接在一个线程中执行该业务逻辑,完事儿后在finally 中调用ThreadLocal的remove方法。
改动后因为增加了其他的业务逻辑,将原查询订单列表的逻辑与新增的其他逻辑分成两个异步任务进行处理,在子任务1中任务完成后有ThreadLocal的remove方法调用,但是把原来在外层的finally 中的remove调用给干掉了。
- 使用的ThreadLocal,子线程不会自动传递该变量;所以改动后的子任务1中,IS_HIGHVENDER.get()始终为初始值false;
- 子任务内的remove只会影响到子线程内的变量,不会影响到外层的ThreadLocal;
- 外层的remove去掉后,外层的ThreadLocal中的值只能通过判断符合v_set.contains(v)条件才会更改,不然就保持是之前的值。
3.回顾
- 记录的时候离事发已经过去一个多月了,上面的伪代码是真的伪,只能按大概记忆编一下了,不保真。
- 这个业务改动前不是我写的,不清楚之前的逻辑;改动后也不是我写的,也不清楚改动后的逻辑,所以有点儿麻烦。
- 收到事故反馈时,前两天才有过上线,就是上线的这个改动,所以排查范围一下就缩小了。
- 事故现象是查到了不该查到的订单,近一步缩小范围到过滤逻辑上。
- 其他过滤条件都是针对订单本身某些属性做的判断,可能性不大。
- 一番排除下来这个ThreadLocal变量嫌疑就比较大了。
- 使用的是ThreadLocal,不是如InheritableThreadLocal这种可在线程间复制传递的,这好像并不是主因。
- 又发现IS_HIGHVENDER这个变量,只有一处调了set方法,一处调了remove方法,多处调了get方法。这个只有一处remove就感觉有点怪了,毕竟现在两个线程中都用到了。
- 外层的remove被干掉了,没有清除它的值,这可能性是极大的。
4.终
最初为啥要使用 ThreadLocal来存储这个变量已经不可考了,看起来貌似没太大必要使用ThreadLocal。
最终找人修复后,看了下修复结果:去除了该ThreadLocal变量,直接 boolean isHighvender = v_set.contains(v); 将isHighvender 变量传递下去进行过滤逻辑判断。
