]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: stream: use atomic increments for the request counter
authorWilly Tarreau <w@1wt.eu>
Wed, 5 Sep 2018 14:21:29 +0000 (16:21 +0200)
committerWilly Tarreau <w@1wt.eu>
Wed, 5 Sep 2018 14:30:19 +0000 (16:30 +0200)
The request counter is incremented when creating a new stream and when
resetting a stream, preparing for a new request. Unfortunately during
the thread migration this was missed, leading to non-atomic increments
in case threads are in use. The most visible side effect is that two
requests may have the same ID from time to time in the logs. However
the SPOE also uses this ID to route responses back to the stream so it
may also lead to occasional spurious SPOE timeouts.

Note that it still doesn't guarantee temporal unicity in the stream
identifiers since a long and a short connection could technically use
the same ID. The likeliness that this happens at the same time is almost
null (roughly threads*runqueue_depth/2^32 that it happens in the same
poll loop), but it will have to be addressed later anyway.

This patch must be backported to 1.8 with the other one it relies on
("MINOR: thread: implement HA_ATOMIC_XADD()").

src/proto_http.c
src/stream.c

index 29b1b68d19b0993b89684cd16e1030a1a4091d21..8ccbc2bcd51a57a8a235341232b7bfeeae372699 100644 (file)
@@ -8364,7 +8364,7 @@ void http_reset_txn(struct stream *s)
        s->target = NULL;
        /* re-init store persistence */
        s->store_count = 0;
-       s->uniq_id = global.req_count++;
+       s->uniq_id = HA_ATOMIC_XADD(&global.req_count, 1);
 
        s->req.flags |= CF_READ_DONTWAIT; /* one read is usually enough */
 
index 0f2ccf010d255fa15300465fc4970c8fa61db369..574c02f3df91ec558dcb1c8f98d9ef04ffd25bce 100644 (file)
@@ -152,7 +152,7 @@ struct stream *stream_new(struct session *sess, enum obj_type *origin)
        s->si[0].flags = SI_FL_NONE;
        s->si[1].flags = SI_FL_ISBACK;
 
-       s->uniq_id = global.req_count++;
+       s->uniq_id = HA_ATOMIC_XADD(&global.req_count, 1);
 
        /* OK, we're keeping the stream, so let's properly initialize the stream */
        LIST_INIT(&s->back_refs);