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

4 天前
 supuwoerc

这两天调试别人项目中的一段 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)
            }
        })
2474 次点击
所在节点    JavaScript
39 条回复
irisdev
4 天前
想想也知道存在并发啊。。一个页面发十个请求,浏览器会等十个请求全部完成再加载页面吗。。
chairuosen
4 天前
多个 tab ,或者框架网页,就会出现
crazzy
4 天前
这都是异步,肯定会出现一个请求检查完 isRefreshing 准备赋值前,另一个请求也在检查 isRefreshing 了。

一个异步的整个生命周期中,事件循环会处理其他任务
IWSR
4 天前
axios 设置拦截器会在每个 HTTP 请求发送时被调用,如果你前端代码内存在同时发送多个请求的逻辑那拦截器内的回调会多次触发,所以多次刷新 token 的现象出现不奇怪,解决办法就是增加一个全局变量去标记刷新 token 这个操作是否已经被触发,如果被触发就不执行刷新 token 的逻辑
zhhcnn
4 天前
用 promise 加锁就行了,读 token 之前 await 一下等 promise 返回
geelaw
4 天前
不存在针对 isRefreshing 的竞态条件,原因很简单:JavaScript 没有多线程,而只有 JavaScript 代码会读写 isRefreshing 。

和 worker 也没有任何关系,因为 worker thread 之间不共享对象,要共享数据必须 postMessage 。
supuwoerc
4 天前
@IWSR 代码里面的 isRefreshing 就是全局变量啊🤔️,只是依靠这个变量也卡不住
irisdev
4 天前
@chairuosen #2 老板,多 tab 这种情况把 isrefreshing 放在 localstorage 里面共享一下是不是就行了
supuwoerc
4 天前
@geelaw 但是从请求记录来看确实触发了多次刷新动作,我很疑惑。
JoeJoeJoe
4 天前
虽然 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: 我大概猜的, 佬们轻喷
ccccccc
4 天前
我建议不要搞得那么复杂,过期该重新登录就重新登录。用户根本不 care 重新登录带来的不方便,除非你几分钟就要用户登录一次
supuwoerc
4 天前
@ccccccc 这个是别人的项目我帮着修 bug 的,物流程序,token 有效期只有 5 分钟,用的长短 token 方案,不断重新登录应该是要爆炸的😂
supuwoerc
4 天前
@JoeJoeJoe 应该不是同时发出去,而是同时返回,同时执行到 if (!isRefreshing)的判断才会引发这个问题,我理解是这样的。
chairuosen
4 天前
@irisdev 不是,他这个逻辑是在加锁的刷新线程里最后处理其他等待线程的后续,多 tab 是处理不了的,得改造一下等待的线程轮询锁自己处理后续
geelaw
4 天前
@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 会进入刷新的逻辑。
JoeJoeJoe
4 天前
@supuwoerc #13 我分析错了, 这个 isRefreshing 是个全局的, sorry.
kamilic
4 天前
isRefreshing 肯定是能锁着的,都是单个线程执行的,cursor 和 copilot 说的没错。
我推测这种情况是不是并发发出的时候,出现这种情况:
| ----- req1 / res1 / refresh ----- | ------------------------ isRefreshing = false ------------------------------------ |
| ----- req2 ------------------------------------------------------------- | --------- res2 / isRefreshing = false ------ |

有可能你并发 req1 / req2 都是返回 10003 的,恰好 req1 刷新 token 后状态还原 isRefreshing ,req2 才返回 10003 ,那不就再刷一次喽。
是不是你的 requests 变量要再记录下在 isRefreshing = true 过程中的所有待返回请求?
supuwoerc
4 天前
@geelaw 感谢大佬,user.useLoginStore.getState()是从 zustand 中读存储的 token/refresh_token 的,我复现的情况没有开多个 tab ,单独一个 tab 就偶发的出现多次刷新 token 的行为。
Niphor
4 天前
F12 network 里多看看就知道拉,并发的多个请求+长短不一的响应时间 就会触发这个
LuckyRock
4 天前
假设 token 过期,发了 A 和 B 两个请求,A 请求收到响应之后过期,刷新 token ,刷新 token 的请求先于 B 请求返回,那么此时 isRefreshing 被重置为 false ,然后 B 请求(携带过期 token )返回了,自然就重新走了刷新 token 的逻辑。

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://ex.noerr.eu.org/t/1156990

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX