Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: errchan replace waitgroup #155

Closed
wants to merge 1 commit into from
Closed

Conversation

myloft
Copy link
Contributor

@myloft myloft commented Aug 11, 2022

WaitGroup and SetReadDeadline maybe not close conn immediately, client send Fin close conn will block in TCP_WAIT2 state.

WaitGroup and SetReadDeadline maybe can not close conn immediately, client close conn maybe block in TCP_WAIT2 state
@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

测试发现可能出现 Client 发送 Fin 报文后,relay 不能及时退出,conn 持续处于 TCP_WAIT2 状态,部分依赖 TCP 连接状态的应用不能及时结束(FTP 传输数据后,客户端需要主动关闭 FTP-DATA TCP 连接来告知 FTP Server 传输完成),导致数据发送失败。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

图片

@xjasonlyu
Copy link
Owner

理论上一侧连接结束时,另一侧会在tcp wait timeout之后关闭,这是为了避免某些write操作结束了但是read还没结束就直接退出的情况发生。

目前tcp wait timeout默认为5秒,影响应该不大吧?感觉一侧结束就马上关闭另一侧太过激进了,v2ray里也有这个延迟关闭的实现。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

SetReadDeadline 似乎对阻塞在 Read 操作中的连接可能不会生效,不会在 tcp wait timeout 5s 后超时退出。

@xjasonlyu
Copy link
Owner

SetReadDeadline 似乎对阻塞在 Read 操作中的连接可能不会生效,不会在 tcp wait timeout 5s 后超时退出。

这就有点奇怪了,如果是这样那问题就属于这块的bug了。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

SetReadDeadline 在不同网络协议里的实现不太一样,不一定能保证让已经开始 Read 的连接超时退出。我是在尝试对接 Quic-Go 时发现的,一旦阻塞到 <-s.readChan 后,只要不收到 EOF,这条 Stream 就不会退出了,也就导致了 relay 无法退出,TCP 也就无法退出了。
https://github.com/lucas-clemente/quic-go/blob/master/receive_stream.go

	if deadline.IsZero() {
		<-s.readChan
	} else {
		select {
		case <-s.readChan:
		case <-deadlineTimer.Chan():
			deadlineTimer.SetRead()
		}
	}

@xjasonlyu
Copy link
Owner

那这是不是就属于它SetReadDeadline实现的bug呢?不过tun2socks里的SetReadDeadline都是系统与gVisor的TCP实现,我之前测试是没问题的。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

嗯嗯 是的 gVisor 和 sys 的协议栈都是正确(符合预期)的,我最近在尝试把代理程序和 tun2socks 集成在一起就发现了这个问题,他们的 SetReadDeadline 一般都直接依赖于传输层(Quic、KCP、TCP),而传输层的 SetReadDeadline 的行为就不一定符合预期了。

@xjasonlyu
Copy link
Owner

什么叫集中在一起?是通过tcp连在一起,还是直接整合到一个binary里?前者的话应该也是没问题的。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

什么叫集中在一起?是通过tcp连在一起,还是直接整合到一个binary里?前者的话应该也是没问题的。

整合在一个 binary,通过实现 DialContext、DialUDP 接口进行数据传输。

@xjasonlyu
Copy link
Owner

那就是其他模块的实现问题了,SetReadlDeadline逻辑就是一段时间内没有数据可读就会返回一个io timeout错误的。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

嗯 不考虑和其他模块组成一个 binary 使用,这段逻辑我觉得还是需要重新考虑一下,比如使用 FTP 连接会延迟断开 Close 5s,表现就是执行命令回显延迟 5s / 传输完成后需要等待 5s 才能继续,显得十分卡顿。

@xjasonlyu
Copy link
Owner

那我觉得可能加一个cli的wait timeout的参数差不多就可以解决。

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

那我觉得可能加一个cli的wait timeout的参数差不多就可以解决。

嗯 wait timeout 0 的话,一条流 close 就立刻 close 另一条流,这样就可以兼顾了。

@xjasonlyu
Copy link
Owner

你要PR这个feature吗?

@myloft
Copy link
Contributor Author

myloft commented Aug 11, 2022

你要PR这个feature吗?

好的

@wustwg
Copy link

wustwg commented Sep 5, 2022

这个改动只是不设置超时时间, 等待了一个连接结束就立即返回了,这可能造成协程泄漏吗?
原来关闭不了的连接,仍然关闭不了呀。感觉并没有解决什么问题。

我今天测试的时候也发现 对gvisor生成的tcpconn设置超时时间无效,即使我在read之前设置好,也是无效的。这bug影响太大了。。。

@myloft
Copy link
Contributor Author

myloft commented Sep 5, 2022

@wustwg 立即返回之后,就会 close left 和 right 了,这样两个协程都会退出了。

@xjasonlyu
Copy link
Owner

这个改动只是不设置超时时间, 等待了一个连接结束就立即返回了,这可能造成协程泄漏吗? 原来关闭不了的连接,仍然关闭不了呀。感觉并没有解决什么问题。

我今天测试的时候也发现 对gvisor生成的tcpconn设置超时时间无效,即使我在read之前设置好,也是无效的。这bug影响太大了。。。

gvisor生成的tcpconn设置超时时间无效?我之前测试是没有问题的,SetReadDeadline时间到了会返回的。

@wustwg
Copy link

wustwg commented Sep 5, 2022

所噶 返回后有delay关闭两个连接,确实如此。

不过直接关闭连接、设置超时关闭都可能有问题:

  1. 如 上传文件的场景,服务端可能关闭了写事件,只保留读事件,这时直接关闭或过几秒钟关闭,文件上传可能还没结束。类似的场景还有下载文件

正常情况下代理逻辑应该是:

  1. 如果local连接关闭了,需要把从local读取到的缓冲区数据全部写入到remote之后,再关闭remote
  2. 如果local连接关闭了,不再关心remote的可读事件,可以取消remote->local的拷贝
  3. 如果remote连接关闭了,也需要把从remote读取到的数据全部写入到local之后,再关闭local
  4. 如果remote连接关闭了,不再关心local的可读事件,可以取消local->remote的拷贝

之所以想做这个改造,一是可以解决上面说的问题,二是想通过异步读取网络连接的数据,达到节省缓冲区的目的,看看有没有可能在ios 15系统以下正常运行。
我想这么改造一下,但是没发现go并没有提供异步(类似于select,epoll)操作网络连接的接口,你们有什么建议吗?

@xjasonlyu @myloft

@wustwg
Copy link

wustwg commented Sep 5, 2022

不知道是啥情况,今天我设置了deadline, 代理服务那边的连接没问题,gvisor的就是不触发,很奇怪。
另外我看gvisor最近几个月更新了不少版本,大佬啥时候更新下?

@xjasonlyu
Copy link
Owner

@wustwg 你说的上传场景不存在这个问题,因为只要有一端还在读或写状态,连接就不会关闭。

@wustwg
Copy link

wustwg commented Sep 5, 2022

不是设置了deadline 5s吗?现在没问题是因为这个deadline没生效,如果这个生效,问题就出现了。
我自己用c写一个socket,我是可以关闭写事件,只接收服务端返回的数据的,这没问题吧?对应的go代码在读取local数据时,就会发现local返回了eof.

@wustwg
Copy link

wustwg commented Sep 14, 2022

@myloft 今天测试用git下载代码发现一直下载不下来,卡着进度不动了,我刚开始以为是git代码太多了,后面才想到是不是代理出了问题,就是上面那个改动(一个连接结束后,尽快关闭另外一个连接)导致的,连接关闭的太快了。

@github-actions github-actions bot added the Stale label Dec 17, 2022
@github-actions github-actions bot closed this Dec 24, 2022
@xjasonlyu xjasonlyu removed the Stale label Mar 28, 2023
@xjasonlyu xjasonlyu self-requested a review March 28, 2023 09:32
@xjasonlyu xjasonlyu added bug Something isn't working and removed bug Something isn't working labels Mar 28, 2023
@xjasonlyu xjasonlyu mentioned this pull request Mar 31, 2023
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.

None yet

3 participants