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

TS-4487: fix write_to_net_io issues #673

Closed
wants to merge 2 commits into from
Closed

Conversation

oknet
Copy link
Member

@oknet oknet commented May 27, 2016

No description provided.

}
}

if (needs==0 && !buf.reader()->read_avail()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use read_avail_more_than(0) here for performance as the exact number of bytes is irrelevant.

@shinrich
Copy link
Member

I think the right thing to do is push PR #629. That moves the read/write loops to use the IOBuffer interface instead of the IOBlock interface. I've been holding off on this one until the event loop changes landed since it solved a real problem I had seen. But if other folks are seeing problems in this area the IOBuffer interface code is much easier to reason about and debug.

@oknet
Copy link
Member Author

oknet commented May 28, 2016

vote on PR #629, but the 2nd issue: did not check the change of lock after return from wbe callback, not included.

@shinrich
Copy link
Member

Good point on the lock check. Could have finished the current vio in the signal and then end of mysteriously disabling the next vio.

@oknet
Copy link
Member Author

oknet commented May 30, 2016

Due to the #629 already merged, I will update the patch to fit it.

@zwoop zwoop added this to the 7.0.0 milestone May 31, 2016
@zwoop
Copy link
Contributor

zwoop commented Jun 27, 2016

@oknet This needs a rebase please. @shinrich Can you review please?

@oknet
Copy link
Member Author

oknet commented Jun 28, 2016

@zwoop @shinrich a new PR#754 created based on the newest master branch.

@oknet oknet closed this Jun 28, 2016
@zwoop zwoop modified the milestone: 7.0.0 May 4, 2017
a-canary pushed a commit to a-canary/trafficserver that referenced this pull request Mar 7, 2018
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
This addresses a couple clang-13 issues in our internal Edge Traffic
Server branch. One is a missing declaration of startEvent, the other a
duplicated function definition. Both of these are mistakes I introduced
from our opensource 9.2.x merge a few months ago.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants