Skip to content

Commit 40afa2d

Browse files
committed
daemon: fix DoH with multiple "parallel" queries in one connection
1 parent 38a04f4 commit 40afa2d

File tree

6 files changed

+31
-18
lines changed

6 files changed

+31
-18
lines changed

NEWS

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Knot Resolver 6.0.12 (2025-0m-dd)
33

44
Bugfixes
55
--------
6+
- daemon: fix DoH with multiple "parallel" queries in one connection (#931, !1677)
67
- /management/unix-socket: revert to absolute path (#926, !1664)
78
- fix `tags` when used in /local-data/rules/*/records (!1670)
89
- stats: request latency was very incorrect in some cases (!1676)

daemon/http.c

+8-5
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ struct pl_http_sess_data {
7272
struct protolayer_data h;
7373
struct nghttp2_session *h2;
7474

75-
queue_http_stream streams; /* Streams present in the wire buffer. */
75+
/** Info about streams pending for kr_request creation. */
76+
queue_http_stream streams;
77+
7678
trie_t *stream_write_queues; /* Dictionary of stream data that needs to be freed after write. */
7779
int32_t incomplete_stream;
7880
int32_t last_stream; /* The last used stream - mostly the same as incomplete_stream, but can be used after
@@ -912,12 +914,12 @@ static int pl_http_sess_deinit(struct session2 *session, void *data)
912914
http_free_headers(stream->headers);
913915
queue_pop(http->streams);
914916
}
917+
queue_deinit(http->streams);
915918

916919
trie_apply(http->stream_write_queues, stream_write_data_break_err, NULL);
917920
trie_free(http->stream_write_queues);
918921

919922
http_cleanup_stream(http);
920-
queue_deinit(http->streams);
921923
wire_buf_deinit(&http->wire_buf);
922924
nghttp2_session_del(http->h2);
923925

@@ -989,8 +991,8 @@ static enum protolayer_iter_cb_result pl_http_wrap(
989991
prov.read_callback = read_callback;
990992

991993
struct pl_http_sess_data *http = sess_data;
992-
int32_t stream_id = http->last_stream;
993-
int ret = http_send_response(sess_data, stream_id, &prov, HTTP_STATUS_OK);
994+
int32_t stream_id = ctx->req->qsource.stream_id;
995+
int ret = http_send_response(http, stream_id, &prov, HTTP_STATUS_OK);
994996
if (ret)
995997
return protolayer_break(ctx, ret);
996998

@@ -1022,10 +1024,11 @@ static void pl_http_request_init(struct session2 *session,
10221024
struct http_stream *stream = &queue_head(http->streams);
10231025
req->qsource.stream_id = stream->id;
10241026
if (stream->headers) {
1027+
// the request takes ownership of the referred-to memory
10251028
req->qsource.headers = *stream->headers;
10261029
free(stream->headers);
1027-
stream->headers = NULL;
10281030
}
1031+
queue_pop(http->streams);
10291032
}
10301033

10311034
__attribute__((constructor))

daemon/session2.c

+10-9
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ static int session2_submit(
612612
struct session2 *session,
613613
enum protolayer_direction direction, size_t layer_ix,
614614
struct protolayer_payload payload, const struct comm_info *comm,
615-
protolayer_finished_cb cb, void *baton)
615+
struct kr_request *req, protolayer_finished_cb cb, void *baton)
616616
{
617617
if (session->closing)
618618
return kr_error(ECANCELED);
@@ -647,6 +647,7 @@ static int session2_submit(
647647
.direction = direction,
648648
.layer_ix = layer_ix,
649649
.session = session,
650+
.req = req,
650651
.finished_cb = cb,
651652
.finished_cb_baton = baton
652653
};
@@ -1259,7 +1260,7 @@ int session2_unwrap(struct session2 *s, struct protolayer_payload payload,
12591260
void *baton)
12601261
{
12611262
return session2_submit(s, PROTOLAYER_UNWRAP,
1262-
0, payload, comm, cb, baton);
1263+
0, payload, comm, NULL/*req*/, cb, baton);
12631264
}
12641265

12651266
int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
@@ -1272,16 +1273,16 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
12721273
if (kr_fails_assert(ok)) // not found or "last layer"
12731274
return kr_error(EINVAL);
12741275
return session2_submit(s, PROTOLAYER_UNWRAP,
1275-
layer_ix + 1, payload, comm, cb, baton);
1276+
layer_ix + 1, payload, comm, NULL/*req*/, cb, baton);
12761277
}
12771278

12781279
int session2_wrap(struct session2 *s, struct protolayer_payload payload,
1279-
const struct comm_info *comm, protolayer_finished_cb cb,
1280-
void *baton)
1280+
const struct comm_info *comm, struct kr_request *req,
1281+
protolayer_finished_cb cb, void *baton)
12811282
{
12821283
return session2_submit(s, PROTOLAYER_WRAP,
12831284
protolayer_grps[s->proto].num_layers - 1,
1284-
payload, comm, cb, baton);
1285+
payload, comm, req, cb, baton);
12851286
}
12861287

12871288
int session2_wrap_after(struct session2 *s, enum protolayer_type protocol,
@@ -1293,7 +1294,7 @@ int session2_wrap_after(struct session2 *s, enum protolayer_type protocol,
12931294
if (kr_fails_assert(layer_ix > 0)) // not found or "last layer"
12941295
return kr_error(EINVAL);
12951296
return session2_submit(s, PROTOLAYER_WRAP, layer_ix - 1,
1296-
payload, comm, cb, baton);
1297+
payload, comm, NULL/*req*/, cb, baton);
12971298
}
12981299

12991300
static void session2_event_wrap(struct session2 *s, enum protolayer_event_type event, void *baton)
@@ -1644,8 +1645,8 @@ static int session2_transport_pushv(struct session2 *s,
16441645
}
16451646
int ret = session2_wrap(parent,
16461647
protolayer_payload_iovec(iov, iovcnt, iov_short_lived),
1647-
comm, session2_transport_parent_pushv_finished,
1648-
ctx);
1648+
comm, NULL/*req*/,
1649+
session2_transport_parent_pushv_finished, ctx);
16491650
return (ret < 0) ? ret : kr_ok();
16501651

16511652
default:

daemon/session2.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ struct protolayer_iter_ctx {
441441
* automatically - layers may use it to allocate memory. */
442442
knot_mm_t pool;
443443

444+
/** Careful! It's only present sometimes and read-only really.
445+
* TODO: maybe a better mechanism should be designed instead of this. */
446+
struct kr_request *req;
447+
444448
/* callback for when the layer iteration has ended - read-only for layers: */
445449
protolayer_finished_cb finished_cb;
446450
void *finished_cb_baton;
@@ -1087,8 +1091,8 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
10871091
* Returns one of `enum protolayer_ret` or a negative number
10881092
* indicating an error. */
10891093
int session2_wrap(struct session2 *s, struct protolayer_payload payload,
1090-
const struct comm_info *comm, protolayer_finished_cb cb,
1091-
void *baton);
1094+
const struct comm_info *comm, struct kr_request *req,
1095+
protolayer_finished_cb cb, void *baton);
10921096

10931097
/** Same as `session2_wrap`, but looks up the specified `protocol` in the
10941098
* session's assigned protocol group and sends the `payload` to the layer that

daemon/worker.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ static int qr_task_send(struct qr_task *task, struct session2 *session,
667667
struct protolayer_payload payload = protolayer_payload_buffer(
668668
(char *)pkt->wire, pkt->size, false);
669669
payload.ttl = packet_ttl(pkt);
670-
ret = session2_wrap(session, payload, comm, qr_task_wrap_finished, task);
670+
ret = session2_wrap(session, payload, comm, &task->ctx->req, qr_task_wrap_finished, task);
671671

672672
if (ret >= 0) {
673673
session2_touch(session);

lib/resolve.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,11 @@ struct kr_request {
251251
uint32_t price_factor16;
252252
size_t size; /**< query packet size */
253253
int32_t stream_id; /**< HTTP/2 stream ID for DoH requests */
254-
kr_http_header_array_t headers; /**< HTTP/2 headers for DoH requests */
254+
/** HTTP/2 headers for DoH requests
255+
*
256+
* Note that this owns malloc-ed memory inside (outside ->pool).
257+
*/
258+
kr_http_header_array_t headers;
255259
} qsource;
256260
struct {
257261
unsigned rtt; /**< Current upstream RTT */

0 commit comments

Comments
 (0)