Skip to content

Conversation

@patterniha
Copy link
Collaborator

@patterniha patterniha commented Jul 8, 2025

  1. after a sub-connection(session) is closed, we should notify remote peer immediately to close this session.

    This did not happen in some cases.(after session-output is closed)

  2. we should close link.Reader and link.Writer after end of ServerWorker-run.

  3. this part of code:

    if err := buf.Copy(rr, s.output); err != nil {
    buf.Copy(rr, buf.Discard)
    return s.Close(false)
    }
    return nil

    is written incorrectly and needs to be rewritten.

    return s.Close(false) instead of buf.Copy(rr, buf.Discard) !!!

@Fangliding
Copy link
Member

Fangliding commented Jul 8, 2025

如果你真的很闲能不能麻烦去issue里找几个bug修一修
我的意见是真的出现问题再去弄而不是一直refine refine(

@Fangliding Fangliding closed this Jul 8, 2025
@Fangliding
Copy link
Member

Fangliding commented Jul 8, 2025

这个古早bug不是很久以前就修复了吗
我一直在高强度使用mux 从未再发现过资源泄露 浏览器一关下载马上就停了
你声称修复 但是好多次是自己都不知道复现 还有问题到底好没好

@patterniha
Copy link
Collaborator Author

patterniha commented Jul 8, 2025

the code is clear, for example after the session closed in:

s.Close(false)

there is no notify to server that this session is closed!

if you think 2 PR is too much i merge this PR to #4861 and rename that to fix some mux bugs, OK?

@Fangliding
Copy link
Member

Fangliding commented Jul 8, 2025

当server在这个流上发送下一个帧的时候客户端会发现它已经被关闭了 然后发送end

@patterniha
Copy link
Collaborator Author

When the server sends the next frame on this stream, the client will find that it has been closed and send end

Why do we have to wait until the next frame? this is definitely waste.

ok, i merge this to previous PR and we can discuss there, i can explain changes one by one.

@Fangliding
Copy link
Member

我的意见是真的出现问题再去弄而不是一直refine refine(

@patterniha
Copy link
Collaborator Author

My opinion is to fix it when there is a problem instead of refining it all the time.

i just read the mux-code, because many people complained about.

this is probably my last PR about refining and fixing bugs

@Fangliding
Copy link
Member

Fangliding commented Jul 8, 2025

Why do we have to wait until the next frame? this is definitely waste.

实际情况是从sessionManager移除后缓冲区还有没处理完的frame会直接关闭这个连接(就你说的大流量浪费场景这是几乎100%的)

@patterniha
Copy link
Collaborator Author

patterniha commented Jul 8, 2025

right, I neglected fetchInput function.
It's better to always speak logically‍

///

what about number 2 and 3,
it seems 3 is just typo and for number 2 we should close link.writer and link.reader, not?

@Fangliding
Copy link
Member

Fangliding commented Jul 8, 2025

sessionManager.Add不会返回false 它是在run的的defer里才会关闭导致false的
w.link.Reader关掉后它都会最后经过一堆乱七八糟的东西触发回收writer 但是显式关闭会让handle里的write往关闭的writer里写东西然后炸掉

@patterniha
Copy link
Collaborator Author

patterniha commented Jul 8, 2025

sessionManager.Add will not return false. It is closed in the defer of run, which causes false. After the w.link.Reader is closed, it will eventually trigger the recycling of the writer through a bunch of messy things. However, explicit closing will cause the write in the handle to write something to the closed writer and then blow up.

but we close link.writer in client-mux after sessionManager closed, so we should close it in server-mux after sessionManager closed, both side should act same.

also, if ctx.Done() happen we don't even close the link.Reader !

@patterniha patterniha changed the title Mux: fix some bugs deleted Jul 8, 2025
@patterniha patterniha deleted the mux-fix branch July 19, 2025 14:47
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.

2 participants