记一次愚蠢的操作--线程安全问题

前言

只有光头才能变强。

文本已收录至我的GitHub仓库，欢迎Star：https://github.com/ZhongFuCheng3y/3y

记一次在工作中愚蠢的操作，本文关键字：线程安全

（我怎么天天在写Bug啊）--本文适合新手观看

一、交代背景

我这边有一个系统，提供一个RPC接口去发送各种信息（比如短信、邮件、微信）等等渠道。我这边的系统架构是这样的：

概括：service系统提供一个RPC接口，别人调用我提供的接口，我在service系统中对这个消息进行判断、拼接等等业务逻辑，最后会将这个消息放到消息队列里边。sender系统会消费消息队列里边的数据，然后发送消息

例子：小王调用我们的RPC接口，想要发送邮件。我对邮件的参数进行判断和拼装成一个我这边定义好的Task，将这个Task丢到消息队列里边。sender系统消费这个Task，调用java.mail的API完成发送邮件的功能。

小王调用我们这个RPC接口，只要service系统把这个task丢到消息队列里边去，我们就返回response给小王。

• 只要这个task放到了消息队列里边，我们就返回success。所以有的时候，小王会问：“我这明明返回是success啊，怎么我的邮件没发出去呢”  ------(异步)

每发送一封邮件，我们都会将这封邮件的信息入库（保存在MySQL中），在MySQL中我们可以得知这封邮件的发送时间，发送状态等等。

而小王的这些邮件又十分在意是否成功发送出去了，如果发送失败了他那边需要重发。于是，他监听我们DB的binlog，根据binlog的信息来判断是否需要重发。

由于种种的原因，小王希望调用我们RPC接口的时候就能拿到一个唯一的标识好让他去判断这封邮件是成功还是失败

• 显然，入库的Email ID是不可能的（因为他调我们RPC接口，我们将Task放到消息队列就返回了。此时sender系统还没消费呢)

于是，我们这边打算在service系统生成一个messageId，然后返回给他，将这个messageId绑定到Task里边，一直到入库。

二、上钩

上面确定好需求和思路之后，我这边就去看返回给小王的response对象，一看，发现已经有msgId字段了

public class SendResponse {

    // 错误码
    private int errCode;

    // 错误信息
    private String errInfo;

    // messageId
    private long msgId;

}

我搜了一下这个字段的信息ctrl + shift + f，发现这msgId没有被用到啊。一想，这刚好，我来用了。我看了一下用法，发现这边不是直接使用SendResponse的，而是在外面包了一个枚举类，代码大概如下：

public enum Response {

    SUCCESS(1, "success"),
    PARAM_MISSING(2, "param is missing"),
    INVALID_xxxx(3, "xxxx is invalid"),
    INVALID_xxxx(4, "xxxx is invalid"),

    private SendResponse sendResponse;

    private Response(int errCode, String errInfo) {
        sendResponse = new SendResponse();
        sendResponse.setMsgId(0);
        sendResponse.setErrCode(errCode);
        sendResponse.setErrInfo(errInfo);
    }

    public SendResponse getSendResponse() {
        return sendResponse;
    }

}

有了枚举使用起来就很简单了，比如我发现小王某个参数传进来有问题，我反手就是：

Response.PARAM_ERROR

service系统主要做了两件事

• 判断参数/类型，各种业务逻辑有没有问题，将小王带过来的参数封装成Task对象

• 将Task对象放到消息队列里边

要明确的是：等到整一个调用链结束（将Task对象放到消息队列中），才会将sendResponse对象返回出去。而又因为可能要判断的地方有点多，所以我们这边是这样设计了一个Map来存储数据，这个Map贯穿整条链路：

// 首先将sendResponse默认设置为success，也就是代码如下：
map.put("sendResponse",Response.SUCCESS);

// 如果中途某个地方可能有问题了，那我们将Map中sendResponse进行修改
map.put("sendResponse",Response.ERROR);

// 等整条链路完成，从Map拿出sendResponse返回
return map.get("sendResponse");

于是我要做的就是：在将SendResponse返回之前，我生成一个唯一的msgId，并插入到SendResponse对象里边就好了。

Response.getSendResponse().setMsgid(uuid);

这个需求完成得非常快，简单测试了一下也没毛病，就果断上线了。小王用了一阵子也没说有什么问题，于是这个需求就交付了。

三、出现问题

昨天，小王告诉我：“我这边邮件发送失败啦，有msgId，看下是什么原因造成的“

于是我就去捞线上的日志，发现根据他给出的msgId，我这边打出的日志都不是发送邮件的（而是其他Task的日志）。我这就慌了，难道我们这个系统出问题了？

• 心理活动：msgId能够唯一标识这条Task，而小王发给我的msgId，却是别的Task的内容。是不是出大问题啦(错乱消费？数据全乱了？)，惊慌失措

然后，他那边继续补充：

之后发现邮件是发送成功的，但是他拿到部分的msgId是别的Task的，不是邮件的。于是只能先比对剩下的邮件是否有问题，再看看MsgId是什么原因。

四、寻找问题

现有的条件是：

• 那批邮箱发送是成功的

• 小王拿到了别的Task的msgId

所以，判断系统是没问题的，只是msgId在并发的过程中出了问题（拿到其他Task的msgId了）

于是我就去找原因啦，在查代码的时候发现前同事还在Service系统中的某个类留了一个注解@NotThreadSafe。我就觉得肯定是中途哪个地方我没注意到，导致小王拿到了其他Task的msgId。

人肉Debug了一个午休的时间还是没找出来：每个线程都独有一份的操作对象，对象的属性都没有逸出(都在方法内部操作)，跟着整块链路一直传递，直至链路结束。看似没啥毛病啊，怀疑是不是方向错了。

后来，一想，我应该关注msgId生成以及可能会变动的地方就好了呀。才发现，项目里边用的是枚举啊！

// 首先将sendResponse默认设置为success，也就是代码如下：
map.put("sendResponse",Response.SUCCESS);

// 如果中途某个地方可能有问题了，那我们将Map中sendResponse进行修改
map.put("sendResponse",Response.ERROR);

// 把response的msgId的值设置为当前Task绑定的值
map.get("sendResponse").setMsgid(uuid);

// 等整条链路完成，从Map拿出sendResponse返回
return map.get("sendResponse");

醒悟：

• 现在我有50个线程，每个线程在处理数据的时候都会有一个默认的sendResponse对象，这个对象是用枚举来标识Response.SUCCESS。所以，这50个线程都共享着这个sendResponse对象

• 50个线程共享着这个sendResponse对象，每个线程都可以修改sendResponse里边的msgId属性，这就自然是线程不安全的。

• 所以小王能拿到其他Task的msgId（小王的线程设置完msgId之后，还没返回，三歪的线程又更改了一次msgId，导致小王拿到三歪的msgId了）

总结：

• 终于知道为啥当初前同事在代码上保留了msgId属性，但是没有使用这个属性。

• 使用枚举就不应该带有状态的属性(能修改、可变的属性)