[
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15304125#comment-15304125
]
ASF GitHub Bot commented on TS-4487:
------------------------------------
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/673#discussion_r64913617
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -545,57 +545,61 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection
*vc, EThread *thread)
vc->write.triggered = 0;
write_signal_error(nh, vc, (int)-r);
return;
- } else {
- int wbe_event = vc->write_buffer_empty_event; // save so we can clear
if needed.
+ }
- NET_SUM_DYN_STAT(net_write_bytes_stat, r);
+ int wbe_event = vc->write_buffer_empty_event; // save so we can clear if
needed.
- // Remove data from the buffer and signal continuation.
- ink_assert(buf.reader()->read_avail() >= r);
- buf.reader()->consume(r);
- ink_assert(buf.reader()->read_avail() >= 0);
- s->vio.ndone += r;
+ NET_SUM_DYN_STAT(net_write_bytes_stat, r);
- // If the empty write buffer trap is set, clear it.
- if (!(buf.reader()->is_read_avail_more_than(0)))
- vc->write_buffer_empty_event = 0;
+ // Remove data from the buffer and signal continuation.
+ ink_assert(buf.reader()->read_avail() >= r);
+ buf.reader()->consume(r);
+ ink_assert(buf.reader()->read_avail() >= 0);
+ s->vio.ndone += r;
- net_activity(vc, thread);
- // If there are no more bytes to write, signal write complete,
- ink_assert(ntodo >= 0);
- if (s->vio.ntodo() <= 0) {
- write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
- return;
- } else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
+ // If the empty write buffer trap is set, clear it.
+ if (!(buf.reader()->is_read_avail_more_than(0)))
+ vc->write_buffer_empty_event = 0;
+
+ net_activity(vc, thread);
+ // If there are no more bytes to write, signal write complete,
+ ink_assert(ntodo >= 0);
+ int e = 0;
+ if (s->vio.ntodo() <= 0) {
+ write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
+ return;
+ } else {
+ if (!signalled) {
+ e = VC_EVENT_WRITE_READY;
+ } else if (wbe_event != vc->write_buffer_empty_event) {
// @a signalled means we won't send an event, and the event values
differing means we
// had a write buffer trap and cleared it, so we need to send it now.
- if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
- return;
- } else if (!signalled) {
- if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT)
{
- return;
- }
-
- // change of lock... don't look at shared variables!
- if (lock.get_mutex() != s->vio.mutex.get()) {
- write_reschedule(nh, vc);
- return;
- }
+ e = wbe_event;
}
-
- if (!buf.reader()->read_avail()) {
- write_disable(nh, vc);
+ }
+ if (e) {
+ if (write_signal_and_update(e, vc) != EVENT_CONT) {
return;
}
- if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
+ // change of lock... don't look at shared variables!
+ if (lock.get_mutex() != s->vio.mutex.get()) {
write_reschedule(nh, vc);
+ return;
}
- if ((needs & EVENTIO_READ) == EVENTIO_READ) {
- read_reschedule(nh, vc);
- }
+ }
+
+ if (needs==0 && !buf.reader()->read_avail()) {
--- End diff --
Should use `read_avail_more_than(0)` here for performance as the exact
number of bytes is irrelevant.
> Don't reschedule read depend on needs & did not check the change of lock at
> the return callback with wbe.
> ---------------------------------------------------------------------------------------------------------
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
> Issue Type: Improvement
> Components: SSL
> Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written,
> buf, needs);
> {code}
> At the end of write_to_net_io,
> {code}
> if (!buf.reader()->read_avail()) { // should check needs==0
> write_disable(nh, vc);
> return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
> write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
> read_reschedule(nh, vc);
> }
> return;
> {code}
> another issue in write_to_net_io(): did not check the change of lock at the
> return callback with wbe.
> {code}
> if (s->vio.ntodo() <= 0) {
> write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
> return;
> } else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
> // @a signalled means we won't send an event, and the event values
> differing means we
> // had a write buffer trap and cleared it, so we need to send it now.
> if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
> return;
> // ============> did not check the change of lock at the return callback
> with wbe.
> } else if (!signalled) {
> if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) {
> return;
> }
> // change of lock... don't look at shared variables!
> if (lock.get_mutex() != s->vio.mutex.get()) {
> write_reschedule(nh, vc);
> return;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
