|
@@ -1,293 +0,0 @@
|
1
|
|
-From 2a4f511b33958b5a09cee2913f1ed9d3210f98f5 Mon Sep 17 00:00:00 2001
|
2
|
|
-From: Willy Tarreau <w@1wt.eu>
|
3
|
|
-Date: Mon, 23 Jun 2014 15:22:31 +0200
|
4
|
|
-Subject: [PATCH 5/5] BUG/MAJOR: session: revert all the crappy client-side
|
5
|
|
- timeout changes
|
6
|
|
-
|
7
|
|
-This is the 3rd regression caused by the changes below. The latest to
|
8
|
|
-date was reported by Finn Arne Gangstad. If a server responds with no
|
9
|
|
-content-length and the client's FIN is never received, either we leak
|
10
|
|
-the client-side FD or we spin at 100% CPU if timeout client-fin is set.
|
11
|
|
-
|
12
|
|
-Enough is enough. The amount of tricks needed to cover these side-effects
|
13
|
|
-starts to look like used toilet paper stacked over a chocolate cake. I
|
14
|
|
-don't want to eat that cake anymore!
|
15
|
|
-
|
16
|
|
-All this to avoid reporting a server-side timeout when a client stops
|
17
|
|
-uploading data and haproxy expires faster than the server... A lot of
|
18
|
|
-"ifs" resulting in a technically valid log that doesn't always please
|
19
|
|
-users, and whose alternative causes that many issues for all others
|
20
|
|
-users.
|
21
|
|
-
|
22
|
|
-So let's revert this crap merged since 1.5-dev25 :
|
23
|
|
- Revert "CLEANUP: http: don't clear CF_READ_NOEXP twice"
|
24
|
|
- This reverts commit 1592d1e72a4a2d25a554c299ae95a3e6cad80bf1.
|
25
|
|
- Revert "BUG/MEDIUM: http: clear CF_READ_NOEXP when preparing a new transaction"
|
26
|
|
- This reverts commit 77d29029af1c44216b190dd7442964b9d8f45257.
|
27
|
|
- Revert "BUG/MEDIUM: session: don't clear CF_READ_NOEXP if analysers are not called"
|
28
|
|
- This reverts commit 0943757a2144761c60e416b5ed07baa76934f5a4.
|
29
|
|
- Revert "BUG/MEDIUM: http: disable server-side expiration until client has sent the body"
|
30
|
|
- This reverts commit 3bed5e9337fd6eeab0f0006ebefcbe98ee5c4f9f.
|
31
|
|
- Revert "BUG/MEDIUM: http: correctly report request body timeouts"
|
32
|
|
- This reverts commit b9edf8fbecc9d1b5c82794735adcc367a80a4ae2.
|
33
|
|
- Revert "BUG/MEDIUM: http/session: disable client-side expiration only after body"
|
34
|
|
- This reverts commit b1982e27aaff2a92a389a9f1bc847e3bb8fdb4f2.
|
35
|
|
-
|
36
|
|
-If a cleaner AND SAFER way to do something equivalent in 1.6-dev, we *might*
|
37
|
|
-consider backporting it to 1.5, but given the vicious bugs that have surfaced
|
38
|
|
-since, I doubt it will happen any time soon.
|
39
|
|
-
|
40
|
|
-Fortunately, that crap never made it into 1.4 so no backport is needed.
|
41
|
|
-(cherry picked from commit 6f0a7bac282c9b2082dc763977b7721b6b002089)
|
42
|
|
----
|
43
|
|
- src/proto_http.c | 95 ++------------------------------------------------------
|
44
|
|
- src/session.c | 41 ++++++++++++------------
|
45
|
|
- 2 files changed, 23 insertions(+), 113 deletions(-)
|
46
|
|
-
|
47
|
|
-diff --git a/src/proto_http.c b/src/proto_http.c
|
48
|
|
-index 52319a9..878951f 100644
|
49
|
|
---- a/src/proto_http.c
|
50
|
|
-+++ b/src/proto_http.c
|
51
|
|
-@@ -4884,7 +4884,7 @@ void http_end_txn_clean_session(struct session *s)
|
52
|
|
- s->req->cons->conn_retries = 0; /* used for logging too */
|
53
|
|
- s->req->cons->exp = TICK_ETERNITY;
|
54
|
|
- s->req->cons->flags &= SI_FL_DONT_WAKE; /* we're in the context of process_session */
|
55
|
|
-- 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_READ_NOEXP);
|
56
|
|
-+ 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);
|
57
|
|
- s->rep->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);
|
58
|
|
- s->flags &= ~(SN_DIRECT|SN_ASSIGNED|SN_ADDR_SET|SN_BE_ASSIGNED|SN_FORCE_PRST|SN_IGNORE_PRST);
|
59
|
|
- s->flags &= ~(SN_CURR_SESS|SN_REDIRECTABLE|SN_SRV_REUSED);
|
60
|
|
-@@ -5305,13 +5305,6 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
|
61
|
|
- */
|
62
|
|
- msg->msg_state = HTTP_MSG_ERROR;
|
63
|
|
- http_resync_states(s);
|
64
|
|
--
|
65
|
|
-- if (req->flags & CF_READ_TIMEOUT)
|
66
|
|
-- goto cli_timeout;
|
67
|
|
--
|
68
|
|
-- if (req->flags & CF_WRITE_TIMEOUT)
|
69
|
|
-- goto srv_timeout;
|
70
|
|
--
|
71
|
|
- return 1;
|
72
|
|
- }
|
73
|
|
-
|
74
|
|
-@@ -5478,11 +5471,6 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
|
75
|
|
- channel_auto_read(req);
|
76
|
|
- }
|
77
|
|
-
|
78
|
|
-- /* if we received everything, we don't want to expire anymore */
|
79
|
|
-- if (msg->msg_state == HTTP_MSG_DONE) {
|
80
|
|
-- req->flags |= CF_READ_NOEXP;
|
81
|
|
-- req->rex = TICK_ETERNITY;
|
82
|
|
-- }
|
83
|
|
- return 0;
|
84
|
|
- }
|
85
|
|
- }
|
86
|
|
-@@ -5592,68 +5580,6 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
|
87
|
|
- s->flags |= SN_FINST_D;
|
88
|
|
- }
|
89
|
|
- return 0;
|
90
|
|
--
|
91
|
|
-- cli_timeout:
|
92
|
|
-- if (!(s->flags & SN_ERR_MASK))
|
93
|
|
-- s->flags |= SN_ERR_CLITO;
|
94
|
|
--
|
95
|
|
-- if (!(s->flags & SN_FINST_MASK)) {
|
96
|
|
-- if (txn->rsp.msg_state < HTTP_MSG_ERROR)
|
97
|
|
-- s->flags |= SN_FINST_H;
|
98
|
|
-- else
|
99
|
|
-- s->flags |= SN_FINST_D;
|
100
|
|
-- }
|
101
|
|
--
|
102
|
|
-- if (txn->status > 0) {
|
103
|
|
-- /* Don't send any error message if something was already sent */
|
104
|
|
-- stream_int_retnclose(req->prod, NULL);
|
105
|
|
-- }
|
106
|
|
-- else {
|
107
|
|
-- txn->status = 408;
|
108
|
|
-- stream_int_retnclose(req->prod, http_error_message(s, HTTP_ERR_408));
|
109
|
|
-- }
|
110
|
|
--
|
111
|
|
-- msg->msg_state = HTTP_MSG_ERROR;
|
112
|
|
-- req->analysers = 0;
|
113
|
|
-- s->rep->analysers = 0; /* we're in data phase, we want to abort both directions */
|
114
|
|
--
|
115
|
|
-- session_inc_http_err_ctr(s);
|
116
|
|
-- s->fe->fe_counters.failed_req++;
|
117
|
|
-- s->be->be_counters.failed_req++;
|
118
|
|
-- if (s->listener->counters)
|
119
|
|
-- s->listener->counters->failed_req++;
|
120
|
|
-- return 0;
|
121
|
|
--
|
122
|
|
-- srv_timeout:
|
123
|
|
-- if (!(s->flags & SN_ERR_MASK))
|
124
|
|
-- s->flags |= SN_ERR_SRVTO;
|
125
|
|
--
|
126
|
|
-- if (!(s->flags & SN_FINST_MASK)) {
|
127
|
|
-- if (txn->rsp.msg_state < HTTP_MSG_ERROR)
|
128
|
|
-- s->flags |= SN_FINST_H;
|
129
|
|
-- else
|
130
|
|
-- s->flags |= SN_FINST_D;
|
131
|
|
-- }
|
132
|
|
--
|
133
|
|
-- if (txn->status > 0) {
|
134
|
|
-- /* Don't send any error message if something was already sent */
|
135
|
|
-- stream_int_retnclose(req->prod, NULL);
|
136
|
|
-- }
|
137
|
|
-- else {
|
138
|
|
-- txn->status = 504;
|
139
|
|
-- stream_int_retnclose(req->prod, http_error_message(s, HTTP_ERR_504));
|
140
|
|
-- }
|
141
|
|
--
|
142
|
|
-- msg->msg_state = HTTP_MSG_ERROR;
|
143
|
|
-- req->analysers = 0;
|
144
|
|
-- s->rep->analysers = 0; /* we're in data phase, we want to abort both directions */
|
145
|
|
--
|
146
|
|
-- s->be->be_counters.failed_resp++;
|
147
|
|
-- if (objt_server(s->target)) {
|
148
|
|
-- objt_server(s->target)->counters.failed_resp++;
|
149
|
|
-- health_adjust(objt_server(s->target), HANA_STATUS_HTTP_READ_TIMEOUT);
|
150
|
|
-- }
|
151
|
|
-- return 0;
|
152
|
|
- }
|
153
|
|
-
|
154
|
|
- /* This stream analyser waits for a complete HTTP response. It returns 1 if the
|
155
|
|
-@@ -5821,11 +5747,8 @@ int http_wait_for_response(struct session *s, struct channel *rep, int an_bit)
|
156
|
|
- return 0;
|
157
|
|
- }
|
158
|
|
-
|
159
|
|
-- /* read/write timeout : return a 504 to the client.
|
160
|
|
-- * The write timeout may happen when we're uploading POST
|
161
|
|
-- * data that the server is not consuming fast enough.
|
162
|
|
-- */
|
163
|
|
-- else if (rep->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) {
|
164
|
|
-+ /* read timeout : return a 504 to the client. */
|
165
|
|
-+ else if (rep->flags & CF_READ_TIMEOUT) {
|
166
|
|
- if (msg->err_pos >= 0)
|
167
|
|
- http_capture_bad_message(&s->be->invalid_rep, s, msg, msg->msg_state, s->fe);
|
168
|
|
- else if (txn->flags & TX_NOT_FIRST)
|
169
|
|
-@@ -5921,12 +5844,6 @@ int http_wait_for_response(struct session *s, struct channel *rep, int an_bit)
|
170
|
|
- return 0;
|
171
|
|
- }
|
172
|
|
-
|
173
|
|
-- /* we don't want to expire on the server side first until the client
|
174
|
|
-- * has sent all the expected message body.
|
175
|
|
-- */
|
176
|
|
-- if (txn->req.msg_state >= HTTP_MSG_BODY && txn->req.msg_state < HTTP_MSG_DONE)
|
177
|
|
-- rep->flags |= CF_READ_NOEXP;
|
178
|
|
--
|
179
|
|
- channel_dont_close(rep);
|
180
|
|
- rep->flags |= CF_READ_DONTWAIT; /* try to get back here ASAP */
|
181
|
|
- return 0;
|
182
|
|
-@@ -6742,12 +6659,6 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
|
183
|
|
- }
|
184
|
|
- return 1;
|
185
|
|
- }
|
186
|
|
--
|
187
|
|
-- /* if we received everything, we don't want to expire anymore */
|
188
|
|
-- if (msg->msg_state == HTTP_MSG_DONE) {
|
189
|
|
-- res->flags |= CF_READ_NOEXP;
|
190
|
|
-- res->rex = TICK_ETERNITY;
|
191
|
|
-- }
|
192
|
|
- return 0;
|
193
|
|
- }
|
194
|
|
- }
|
195
|
|
-diff --git a/src/session.c b/src/session.c
|
196
|
|
-index f828d9c..e26f5ad 100644
|
197
|
|
---- a/src/session.c
|
198
|
|
-+++ b/src/session.c
|
199
|
|
-@@ -1636,7 +1636,6 @@ struct task *process_session(struct task *t)
|
200
|
|
- unsigned int rq_prod_last, rq_cons_last;
|
201
|
|
- unsigned int rp_cons_last, rp_prod_last;
|
202
|
|
- unsigned int req_ana_back;
|
203
|
|
-- unsigned int rq_oneshot, rp_oneshot;
|
204
|
|
-
|
205
|
|
- //DPRINTF(stderr, "%s:%d: cs=%d ss=%d(%d) rqf=0x%08x rpf=0x%08x\n", __FUNCTION__, __LINE__,
|
206
|
|
- // s->si[0].state, s->si[1].state, s->si[1].err_type, s->req->flags, s->rep->flags);
|
207
|
|
-@@ -1644,13 +1643,9 @@ struct task *process_session(struct task *t)
|
208
|
|
- /* this data may be no longer valid, clear it */
|
209
|
|
- memset(&s->txn.auth, 0, sizeof(s->txn.auth));
|
210
|
|
-
|
211
|
|
-- /* These flags must explicitly be set every time by the analysers who
|
212
|
|
-- * need them, but we won't always call them (eg: during a connection
|
213
|
|
-- * retry). So we need to keep them and only clear them if we're sure
|
214
|
|
-- * to call the analysers.
|
215
|
|
-- */
|
216
|
|
-- rq_oneshot = s->req->flags & (CF_READ_NOEXP | CF_WAKE_WRITE);
|
217
|
|
-- rp_oneshot = s->rep->flags & (CF_READ_NOEXP | CF_WAKE_WRITE);
|
218
|
|
-+ /* This flag must explicitly be set every time */
|
219
|
|
-+ s->req->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE);
|
220
|
|
-+ s->rep->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE);
|
221
|
|
-
|
222
|
|
- /* Keep a copy of req/rep flags so that we can detect shutdowns */
|
223
|
|
- rqf_last = s->req->flags & ~CF_MASK_ANALYSER;
|
224
|
|
-@@ -1831,8 +1826,6 @@ struct task *process_session(struct task *t)
|
225
|
|
- s->si[1].state != rq_cons_last) {
|
226
|
|
- unsigned int flags = s->req->flags;
|
227
|
|
-
|
228
|
|
-- s->req->flags &= ~rq_oneshot;
|
229
|
|
-- rq_oneshot = 0;
|
230
|
|
- if (s->req->prod->state >= SI_ST_EST) {
|
231
|
|
- int max_loops = global.tune.maxpollevents;
|
232
|
|
- unsigned int ana_list;
|
233
|
|
-@@ -1986,13 +1979,11 @@ struct task *process_session(struct task *t)
|
234
|
|
- /* Analyse response */
|
235
|
|
-
|
236
|
|
- if (((s->rep->flags & ~rpf_last) & CF_MASK_ANALYSER) ||
|
237
|
|
-- ((s->rep->flags ^ rpf_last) & CF_MASK_STATIC) ||
|
238
|
|
-- s->si[0].state != rp_cons_last ||
|
239
|
|
-- s->si[1].state != rp_prod_last) {
|
240
|
|
-+ (s->rep->flags ^ rpf_last) & CF_MASK_STATIC ||
|
241
|
|
-+ s->si[0].state != rp_cons_last ||
|
242
|
|
-+ s->si[1].state != rp_prod_last) {
|
243
|
|
- unsigned int flags = s->rep->flags;
|
244
|
|
-
|
245
|
|
-- s->rep->flags &= ~rp_oneshot;
|
246
|
|
-- rp_oneshot = 0;
|
247
|
|
- if ((s->rep->flags & CF_MASK_ANALYSER) &&
|
248
|
|
- (s->rep->analysers & AN_REQ_WAIT_HTTP)) {
|
249
|
|
- /* Due to HTTP pipelining, the HTTP request analyser might be waiting
|
250
|
|
-@@ -2186,9 +2177,6 @@ struct task *process_session(struct task *t)
|
251
|
|
- channel_auto_close(s->req);
|
252
|
|
- buffer_flush(s->req->buf);
|
253
|
|
-
|
254
|
|
-- s->req->flags &= ~rq_oneshot;
|
255
|
|
-- rq_oneshot = 0;
|
256
|
|
--
|
257
|
|
- /* We'll let data flow between the producer (if still connected)
|
258
|
|
- * to the consumer (which might possibly not be connected yet).
|
259
|
|
- */
|
260
|
|
-@@ -2344,9 +2332,6 @@ struct task *process_session(struct task *t)
|
261
|
|
- channel_auto_close(s->rep);
|
262
|
|
- buffer_flush(s->rep->buf);
|
263
|
|
-
|
264
|
|
-- s->rep->flags &= ~rp_oneshot;
|
265
|
|
-- rp_oneshot = 0;
|
266
|
|
--
|
267
|
|
- /* We'll let data flow between the producer (if still connected)
|
268
|
|
- * to the consumer.
|
269
|
|
- */
|
270
|
|
-@@ -2496,6 +2481,20 @@ struct task *process_session(struct task *t)
|
271
|
|
- s->si[0].flags &= ~(SI_FL_ERR|SI_FL_EXP);
|
272
|
|
- s->si[1].flags &= ~(SI_FL_ERR|SI_FL_EXP);
|
273
|
|
-
|
274
|
|
-+ /* Trick: if a request is being waiting for the server to respond,
|
275
|
|
-+ * and if we know the server can timeout, we don't want the timeout
|
276
|
|
-+ * to expire on the client side first, but we're still interested
|
277
|
|
-+ * in passing data from the client to the server (eg: POST). Thus,
|
278
|
|
-+ * we can cancel the client's request timeout if the server's
|
279
|
|
-+ * request timeout is set and the server has not yet sent a response.
|
280
|
|
-+ */
|
281
|
|
-+
|
282
|
|
-+ if ((s->rep->flags & (CF_AUTO_CLOSE|CF_SHUTR)) == 0 &&
|
283
|
|
-+ (tick_isset(s->req->wex) || tick_isset(s->rep->rex))) {
|
284
|
|
-+ s->req->flags |= CF_READ_NOEXP;
|
285
|
|
-+ s->req->rex = TICK_ETERNITY;
|
286
|
|
-+ }
|
287
|
|
-+
|
288
|
|
- /* When any of the stream interfaces is attached to an applet,
|
289
|
|
- * we have to call it here. Note that this one may wake the
|
290
|
|
- * task up again. If at least one applet was called, the current
|
291
|
|
-1.8.5.5
|
292
|
|
-
|