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

jiezi

5 年前

只有光头才能变强。

文本已收录至我的 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 属性，但是没有使用这个属性。
使用枚举就不应该带 有状态的属性(能修改、可变的属性)

乐于输出干货的 Java 技术公众号：Java3y。公众号内 有 200 多篇原创 技术文章、海量视频资源、精美脑图，关注即可获取！

觉得我的文章写得不错，点赞！

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

前言

一、交代背景

二、上钩

三、出现问题

四、寻找问题

最后