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 #1494 socket read阻塞问题 #1495

Merged
merged 2 commits into from
Oct 29, 2021
Merged

Conversation

lvzixun
Copy link
Contributor

@lvzixun lvzixun commented Oct 28, 2021

在socket init 时执行init_responsefunc 时,有可能会导致后面的header也收到了,所以在后续的readfunc 中第一次要从tls中读取下数据再尝试去从socket 读,避免阻塞。

@cloudwu
Copy link
Owner

cloudwu commented Oct 28, 2021

这个改法有点奇怪:

  1. 似乎只有第一次调用的时候,才会额外读一下 tls_ctx:read() 。但其实,理论上每次的 local ds = readfunc(sz) 都可能读多了,导致下一次的读 socket 时并无数据,但 tls_ctx 中还有内容。
  2. 如果真的只需要在第一次调用前读,看起来就不需要每次调用都判断一次了。似乎一开始读一次即可。

@cloudwu
Copy link
Owner

cloudwu commented Oct 28, 2021

关于上面的 1 , 我不是很确认是不是真的只有握手阶段才有此问题,而之后不再会发生完整的 tls 数据未从 ctx 中读出的情况。

@lvzixun
Copy link
Contributor Author

lvzixun commented Oct 28, 2021

关于上面的 1 , 我不是很确认是不是真的只有握手阶段才有此问题,而之后不再会发生完整的 tls 数据未从 ctx 中读出的情况。
这个是只有在握手阶段才需要尝试读一次的。

  1. 我一开始也想在read_buff 定义的时候尝试读一次。但是这个时候init可能没有被执行,所以就在读的时候判断下了

@cloudwu
Copy link
Owner

cloudwu commented Oct 28, 2021

不知道通过在 readfunc 上做文章能不能达到同样效果:

local function readfunc ()
  readfunc = socket.readfunc(fd)
  return ""
end

如此可以节省每次调用中的一次判断。

@cloudwu
Copy link
Owner

cloudwu commented Oct 29, 2021

@lvzixun 有无打算改改试试?还是先合并这个?

另外,https://github.com/cloudwu/skynet/blob/master/lualib/http/tlshelper.lua#L52 此处的 sz 看起来应当删掉。

@lvzixun
Copy link
Contributor Author

lvzixun commented Oct 29, 2021

@lvzixun 有无打算改改试试?还是先合并这个?

另外,https://github.com/cloudwu/skynet/blob/master/lualib/http/tlshelper.lua#L52 此处的 sz 看起来应当删掉。

恩。我这边把sz 还有 readfunc 都给改了。

@cloudwu cloudwu merged commit e35cad3 into cloudwu:master Oct 29, 2021
@lvzixun lvzixun deleted the bugfix.tls_read branch October 29, 2021 06:08
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