MEDIUM: channel: merge back flags CF_WRITE_PARTIAL and CF_WRITE_EVENT
The behaviour of the flag CF_WRITE_PARTIAL was modified by commit
95fad5ba4 ("BUG/MAJOR: stream-int: don't re-arm recv if send fails") due
to a situation where it could trigger an immediate wake up of the other
side, both acting in loops via the FD cache. This loss has caused the
need to introduce CF_WRITE_EVENT as commit c5a9d5bf, to replace it, but
both flags express more or less the same thing and this distinction
creates a lot of confusion and complexity in the code.
Since the FD cache now acts via tasklets, the issue worked around in the
first patch no longer exists, so it's more than time to kill this hack
and to restore CF_WRITE_PARTIAL's semantics (i.e.: there has been some
write activity since we last left process_stream).
This patch mostly reverts the two commits above. Only the part making
use of CF_WROTE_DATA instead of CF_WRITE_PARTIAL to detect the loss of
data upon connection setup was kept because it's more accurate and
better suited.
diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c
index a9e0909..ae885d6 100644
--- a/contrib/debug/flags.c
+++ b/contrib/debug/flags.c
@@ -72,7 +72,6 @@
SHOW_FLAG(f, CF_ISRESP);
SHOW_FLAG(f, CF_FLT_ANALYZE);
- SHOW_FLAG(f, CF_WRITE_EVENT);
SHOW_FLAG(f, CF_WAKE_ONCE);
SHOW_FLAG(f, CF_NEVER_WAIT);
SHOW_FLAG(f, CF_SEND_DONTWAIT);
diff --git a/include/proto/channel.h b/include/proto/channel.h
index bf84a16..56bf5cf 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -459,7 +459,7 @@
unlikely(tick_is_expired(chn->rex, now_ms)))
chn->flags |= CF_READ_TIMEOUT;
- if (likely(!(chn->flags & (CF_SHUTW|CF_WRITE_TIMEOUT|CF_WRITE_ACTIVITY|CF_WRITE_EVENT))) &&
+ if (likely(!(chn->flags & (CF_SHUTW|CF_WRITE_TIMEOUT|CF_WRITE_ACTIVITY))) &&
unlikely(tick_is_expired(chn->wex, now_ms)))
chn->flags |= CF_WRITE_TIMEOUT;
@@ -772,7 +772,7 @@
c_realign_if_empty(chn);
/* notify that some data was written to the SI from the buffer */
- chn->flags |= CF_WRITE_PARTIAL | CF_WRITE_EVENT;
+ chn->flags |= CF_WRITE_PARTIAL;
}
/* Tries to copy chunk <chunk> into the channel's buffer after length controls.
diff --git a/include/types/channel.h b/include/types/channel.h
index 7879b12..f7ddd1a 100644
--- a/include/types/channel.h
+++ b/include/types/channel.h
@@ -117,11 +117,11 @@
#define CF_WAKE_ONCE 0x10000000 /* pretend there is activity on this channel (one-shoot) */
#define CF_FLT_ANALYZE 0x20000000 /* at least one filter is still analyzing this channel */
-#define CF_WRITE_EVENT 0x40000000 /* write activity not processed yet by the stream */
+/* unused: 0x40000000 */
#define CF_ISRESP 0x80000000 /* 0 = request channel, 1 = response channel */
/* Masks which define input events for stream analysers */
-#define CF_MASK_ANALYSER (CF_READ_ATTACHED|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_ANA_TIMEOUT|CF_WRITE_ACTIVITY|CF_WAKE_ONCE|CF_WRITE_EVENT)
+#define CF_MASK_ANALYSER (CF_READ_ATTACHED|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_ANA_TIMEOUT|CF_WRITE_ACTIVITY|CF_WAKE_ONCE)
/* Mask for static flags which cause analysers to be woken up when they change */
#define CF_MASK_STATIC (CF_SHUTR|CF_SHUTW|CF_SHUTR_NOW|CF_SHUTW_NOW)
diff --git a/src/proto_http.c b/src/proto_http.c
index e0ae82e..8ac4c01 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3694,7 +3694,7 @@
s->si[1].exp = TICK_ETERNITY;
s->si[1].flags &= SI_FL_ISBACK | SI_FL_DONT_WAKE; /* we're in the context of process_stream */
s->req.flags &= ~(CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CONNECT|CF_WRITE_ERROR|CF_STREAMER|CF_STREAMER_FAST|CF_NEVER_WAIT|CF_WAKE_CONNECT|CF_WROTE_DATA);
- s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ATTACHED|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_PARTIAL|CF_NEVER_WAIT|CF_WROTE_DATA|CF_WRITE_EVENT);
+ s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ATTACHED|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_PARTIAL|CF_NEVER_WAIT|CF_WROTE_DATA);
s->flags &= ~(SF_DIRECT|SF_ASSIGNED|SF_ADDR_SET|SF_BE_ASSIGNED|SF_FORCE_PRST|SF_IGNORE_PRST);
s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED);
s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP);
diff --git a/src/stream.c b/src/stream.c
index 8df5c75..ad1ccbe 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -628,7 +628,7 @@
if (!(req->flags & CF_WROTE_DATA) &&
unlikely((rep->flags & CF_SHUTW) ||
((req->flags & CF_SHUTW_NOW) && /* FIXME: this should not prevent a connection from establishing */
- ((!(req->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)) && channel_is_empty(req)) ||
+ ((!(req->flags & CF_WRITE_ACTIVITY) && channel_is_empty(req)) ||
((s->be->options & PR_O_ABRT_CLOSE) && !(s->si[0].flags & SI_FL_CLEAN_ABRT)))))) {
/* give up */
si_shutw(si);
@@ -639,7 +639,7 @@
}
/* we need to wait a bit more if there was no activity either */
- if (!(req->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)))
+ if (!(req->flags & CF_WRITE_ACTIVITY))
return 1;
/* OK, this means that a connection succeeded. The caller will be
@@ -1759,7 +1759,7 @@
*/
if (!((req->flags | res->flags) &
(CF_SHUTR|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_SHUTW|
- CF_WRITE_ACTIVITY|CF_WRITE_EVENT|CF_WRITE_TIMEOUT|CF_ANA_TIMEOUT)) &&
+ CF_WRITE_ACTIVITY|CF_WRITE_TIMEOUT|CF_ANA_TIMEOUT)) &&
!((si_f->flags | si_b->flags) & (SI_FL_EXP|SI_FL_ERR)) &&
((s->pending_events & TASK_WOKEN_ANY) == TASK_WOKEN_TIMER)) {
si_f->flags &= ~SI_FL_DONT_WAKE;
@@ -2477,8 +2477,8 @@
if (si_b->state == SI_ST_EST)
si_update(si_b);
- req->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED|CF_WRITE_EVENT);
- res->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED|CF_WRITE_EVENT);
+ req->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED);
+ res->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_WRITE_NULL|CF_WRITE_PARTIAL|CF_READ_ATTACHED);
si_f->prev_state = si_f->state;
si_b->prev_state = si_b->state;
si_f->flags &= ~(SI_FL_ERR|SI_FL_EXP);
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 59827ec..e585b1f 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -496,10 +496,6 @@
* the buffer is full. We must not stop based on input data alone because
* an HTTP parser might need more data to complete the parsing.
*/
-
- /* ensure it's only set if a write attempt has succeeded */
- ic->flags &= ~CF_WRITE_PARTIAL;
-
if (!channel_is_empty(ic) &&
(si_opposite(si)->flags & SI_FL_WAIT_DATA) &&
(ci_data(ic) == 0 || ic->pipe)) {
@@ -544,7 +540,7 @@
/* changes on the consumption side */
(oc->flags & (CF_WRITE_NULL|CF_WRITE_ERROR)) ||
- ((oc->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)) &&
+ ((oc->flags & CF_WRITE_ACTIVITY) &&
((oc->flags & CF_SHUTW) ||
((oc->flags & CF_WAKE_WRITE) &&
(si_opposite(si)->state != SI_ST_EST ||
@@ -641,13 +637,10 @@
if (si_oc(si)->flags & CF_SHUTW)
return 1;
- /* ensure it's only set if a write attempt has succeeded */
- oc->flags &= ~CF_WRITE_PARTIAL;
-
if (oc->pipe && conn->xprt->snd_pipe && conn->mux->snd_pipe) {
ret = conn->mux->snd_pipe(cs, oc->pipe);
if (ret > 0) {
- oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA | CF_WRITE_EVENT;
+ oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA;
did_send = 1;
}
@@ -696,7 +689,7 @@
ret = cs->conn->mux->snd_buf(cs, &oc->buf, co_data(oc), send_flag);
if (ret > 0) {
did_send = 1;
- oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA | CF_WRITE_EVENT;
+ oc->flags |= CF_WRITE_PARTIAL | CF_WROTE_DATA;
co_set_data(oc, co_data(oc) - ret);
c_realign_if_empty(oc);
@@ -965,9 +958,6 @@
struct channel *oc = si_oc(si);
struct conn_stream *cs = __objt_cs(si->end);
- /* ensure it's only set if a write attempt has succeeded */
- oc->flags &= ~CF_WRITE_PARTIAL;
-
if (unlikely(si->state > SI_ST_EST || (oc->flags & CF_SHUTW)))
return;