这段前端代码存在并发读取竞态的问题吗？

这两天调试别人项目中的一段 js 代码，作用是刷新 token ，但是验证下来发现有很小的几率会触发多次刷新 token 的动作(下面代码中的 FIXME 位置)，特别是 Promise.all 去发送一批请求的时候，我 google 了一圈，没研究明白，因为复现起来很困难，所以请教大家，代码中读取 isRefreshing 是安全的吗？我让 cursor 和 copilot 解释都是说 js 不启用 worker 是不存在并发问题的，但是从结果来看，确实有不止一个请求进入了刷新 token 的分支，我把这个情况描述完，cursor 让我引入 sync-mutex 加锁，和一开始的解释完全不一样，我在 StackOverflow 和 medium 中也找到几篇类似的文章，都是借助了防抖/记忆函数来解决，实在弄不清楚这块读写 isRefreshing 到底是不是安全的。

还看到了一篇锁的文章，感觉很类似我遇到的这个问题： https://jackpordi.com/posts/locks-in-js-because-why-not

伪代码如下：

let isRefreshing = false // 标记是否正在刷新 token 
let requests: Array<(token: string, err?: string) => void> = [] // 需要重试的请求列表

client.interceptors.response.use((response: AxiosResponse) => {
            const { config, status } = response
            const { code } = response.data
            if (status >= 500) {
                return Promise.reject("服务器错误")
            } else if (code == 10003) {
                // access token 过期，尝试刷新 token
                const { refreshToken } = user.useLoginStore.getState()
                if (refreshToken) {
                    // FIXME： ?? 存在并发读取 isRefreshing 为 false 导致发出多次刷新 token 的请求
                    if (!isRefreshing) {
                        isRefreshing = true
                        return refreshToken()
                            .then(({ data }) => {
                                const { code } = data
                                if (code === 10000) {
                                    user.useLoginStore.setState((state) => {
                                        state.token = data.data.token
                                    })
                                    config.headers["Authorization"] = data.data.token
                                    const retry = client(config)
                                    requests.forEach((cb) => cb(data.data.token))
                                    requests = []
                                    return retry
                                } else {
                                    return Promise.reject(data.message)
                                }
                            })
                            .catch((err) => {
                                const msg = isError(err) ? err.message : err
                                requests.forEach((cb) => cb("", msg))
                                requests = []
                                publishInvalidTokenEvent(msg)
                            })
                            .finally(() => {
                                isRefreshing = false
                            })
                    } else {
                        return new Promise((resolve, reject) => {
                            requests.push((token: string, err?: string) => {
                                if (err) {
                                    reject(err)
                                } else {
                                    config.headers["Authorization"] = token
                                    resolve(client(config))
                                }
                            })
                        })
                    }
                } else {
                    requests.forEach((cb) => cb("", "登录过期")
                    requests = []
                    publishInvalidTokenEvent("登录过期")
                }
            } else if (code === 10000) {
                return response.data.data
            } else if (code == 10006) {
                // 长 token 失效
                requests.forEach((cb) => cb("", "登录过期")
                requests = []
                publishInvalidTokenEvent("登录过期")
            } else {
                return Promise.reject(response.data.message || response.data.msg)
            }
        })

JoeJoeJoe

11 天前

虽然 js 不开 worker 是单线程的, 但是你的请求是异步的, 也就是说你的刷新 token 也是异步的, 不会阻塞后续的其他请求, 在等待新 token 的时候, 其他的接口如果有返回值回来, 同样, 他们的 token 也是过期的, 也是会触发你的刷新 token 逻辑的.
比如: 现在的 token 是过期的, 同时发送了 A,B,C3 个请求
1. A 请求发送之后, 接口返回 token 过期, 然后刷新 token, 获取到新的 token, 如果这期间 B,C 还没有发出去, 那么是没有问题的.
2. A 请求发送之后, B,C 请求同时发出去了, 那么那么 BC 请求带的 token 也是过期 Token, 也会触发你说的现象.
3. A 请求发送之后, BC 请求还没有发送, A 请求收到了 token 过期的响应, 开始请求刷新 token, 这期间 BC 请求发出去了, 这种场景也会触发你说的现象.

ps: 我大概猜的, 佬们轻喷

geelaw

11 天前

@chairuosen #2 每个 tab 里都有自己的 isRefreshing 。

@crazzy #3 在 if (!isRefreshing) 和 isRefreshing = true 之间不可能有任何 JavaScript 代码执行。

@IWSR #4 这个好像是楼主代码的意思。

@supuwoerc #9

楼主可以多考虑一下读者，尝试简化代码，使之不需要借助外部语境就很容易理解，比如我们不知道 user.useLoginStore.getState() 的状态是否是 session/local 级别的，还是 tab 级别的。如果 user.useLoginStore.getState() 是 session/local 的状态，那么不同的 tab 有自己的 isRefreshing ，当然两个 tab 可以同时看到过期的 token 并分别决定刷新。

@irisdev #8 放在 local/session 都不好，因为用户可以在 tab 1 刷新的时候关闭之，但 tab 1 通过共享的 isRefreshing 锁定了刷新的权力，这会导致 local/session 清空之前不再有任何 tab 会进入刷新的逻辑。

kamilic

11 天前

isRefreshing 肯定是能锁着的，都是单个线程执行的，cursor 和 copilot 说的没错。
我推测这种情况是不是并发发出的时候，出现这种情况：
| ----- req1 / res1 / refresh ----- | ------------------------ isRefreshing = false ------------------------------------ |
| ----- req2 ------------------------------------------------------------- | --------- res2 / isRefreshing = false ------ |

有可能你并发 req1 / req2 都是返回 10003 的，恰好 req1 刷新 token 后状态还原 isRefreshing ，req2 才返回 10003 ，那不就再刷一次喽。
是不是你的 requests 变量要再记录下在 isRefreshing = true 过程中的所有待返回请求？