Skip to content

DNS outbound: Preserve session context for timeout-only connections#6425

Open
Meo597 wants to merge 7 commits into
mainfrom
dns-out-ctx
Open

DNS outbound: Preserve session context for timeout-only connections#6425
Meo597 wants to merge 7 commits into
mainfrom
dns-out-ctx

Conversation

@Meo597

@Meo597 Meo597 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

陈年老 BUG 了,只是没人那么无聊这么配过所以没发现

  • 服务端上,客户端 XUDP 进来到 DNS 出站的流量没法判 isOwnLink() 特定配置会导致多绕一次
  • 如果配了 sendThrough 那么 direct 的查询还会 panic

@Fangliding

Copy link
Copy Markdown
Member

感觉TimeoutOnly换成background就是有问题 把所有value ctx都洗掉了 这样的写法不止一处

@Meo597

Meo597 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

是的,所有的出站遇到 XUDP 都 ctx = bg 是在埋雷

@Fangliding

Copy link
Copy Markdown
Member

我的意思是可以都换成 withoutcancel

@Meo597

Meo597 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

那些其它出站都当 newCtx 给 task.Run 用我之前扫了一眼觉得不会有 BUG 只是以后可能是雷点

我看看都改了他们吧

@Meo597 Meo597 changed the title DNS outbound: Preserve session context for timeout-only connections Outbounds: Preserve session context for timeout-only connections Jul 3, 2026
@RPRX

RPRX commented Jul 3, 2026

Copy link
Copy Markdown
Member

这东西当初我写的,帮我找一下当初的 commit 我回忆一下为什么这么写

@RPRX

RPRX commented Jul 3, 2026

Copy link
Copy Markdown
Member

写代码不留注释真他妈的要命,过个一年半载就忘了逻辑,以后我还是多留注释吧

@Meo597

Meo597 commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

这东西当初我写的,帮我找一下当初的 commit 我回忆一下为什么这么写

be23d5d
#2017 (comment)

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

context.withoutcancel 是不是也是我加的来着,我为啥加它来着,懒得翻历史了

依稀记得好像是备选方案,加了后没用过

@Fangliding

Fangliding commented Jul 8, 2026

Copy link
Copy Markdown
Member

看起来是xudp为了断掉跟上游ctx的生命周期绑定方便其他人接管连接

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

或许当时你写的时候还没有 withoutcancel
这俩是等价的,不放心的话可以 revert 最后那个 commit 因为 dns 肯定是错的

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

就是说看 context 开头感觉是 Go 的函数,我也查了下,不是我加的,23 年 8 月份 Go 1.21 自己加的,我当时写的时候还没有

我是当时看了看 Go 代码,linkname 导出了一个 IndependentCancelCtx 但没用过

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

还有个问题是有这个函数后能不能在 mux dispatch 那里就加上,而不是每个出站加

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

还有 gRPC

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

#2232

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

大概看了一眼,意思不能这么干,你当时故意写的 newCtx
这个 PR 只能动 dns?

gRPC 那里加的 ContextWithTimeoutOnly 看不懂

哦明白了,会被 dialerProxy 后的 outbound 消费,因此不能统一在 mux 里直接弄

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

注释被你找到了但你操作错了,重点是 #2232 (comment)

只想让原始 ctx 控制 dial 超时,不想让它 cancel Copy,但又想让 outbound 自身的超时策略生效

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

虽然一些 inbound 改成 DispatchLink() 后,上游还有没有影响 dial 超时是个问题,可能只剩客户端传导来的 close?

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

要不靠谱点把这些都 revert 只留第一个 commit 吧

闭包里吃的是上游 ctx
从第二个 commit 开始上游的所有 cancel 都被截断了

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

主要是传输层没处理这些东西,如果有 dialerProxy 的话传输层会被 cancel,所以该放哪还是再想想吧

或许应当放弃上游对 dial 超时的控制,毕竟上游用 DisptchLink() 的话可能已经没 timeout 了,只剩其它原因

@Meo597 Meo597 changed the title Outbounds: Preserve session context for timeout-only connections DNS outbound: Preserve session context for timeout-only connections Jul 8, 2026
@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

下个 PR 再说吧,那些地方我没摸过不是完全清楚

newCtx 放在 task.Run 之前而不是立即
之前感觉怪怪的,原来是故意的

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

我倾向于暂时不合并这个 PR,本来就不是导致了严重 bug,至少你先摸清楚了再说

@RPRX

RPRX commented Jul 8, 2026

Copy link
Copy Markdown
Member

总之其它 outbounds 主要是想只是 copy 时不被上游 cancel,所以只在 task.Run 前,在此之前传递的 ctx 不受影响

可能也可以换成这种简化写法?还是直接在 mux 上游处理,还没想好

但是 DNS 这个下面用到了 ctx 里的值,绝对不引入新 bug 的另一种方法是提前读出来

@Fangliding

Copy link
Copy Markdown
Member

你们这个 不被上游 cancel withoutcancel 不是可以解决吗?

@Meo597

Meo597 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

R 主席想要让闭包里面的那些还能被上游 cancel,比如 dial 啥的
这样的话就不能碰那些出站 newCtx 部分了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants