[PATCH 04/11] Port: removed useless msg->cancelled == 0 checks.
Andrew Clayton
andrew at digital-domain.net
Fri Jun 17 02:46:54 UTC 2022
On Thu, 16 Jun 2022 18:25:33 +0100
Andrew Clayton <andrew at digital-domain.net> wrote:
> On Thu, 16 Jun 2022 17:51:30 +0200
> Alejandro Colomar <alx.manpages at gmail.com> wrote:
>
> > This patch makes sense, but then, I guess we should go further and
> > remove that field completely? I don't see it being used at all.
>
> Yes, I think you're right, I'll check again and add it to the patch.
So, this has led down a bit of a rabbit hole!
->cancelled is still used by
1196 fmsg = nxt_port_frag_find(task, port, msg);
...
1202 if (nxt_fast_path(fmsg->cancelled == 0)) {
in src/nxt_port_socket.c::nxt_port_read_msg_process()
From what I can tell fmsg is basically a copy of msg and stored in a
hash table.
So how is msg created? Seems to be generally just created and
set directly on the stack e.g
738 nxt_port_recv_msg_t msg;
...
761 msg.fd[0] = -1;
762 msg.fd[1] = -1;
...
775 msg.buf = b;
776 msg.size = n;
777
778 nxt_port_read_msg_process(task, port, &msg);
in src/nxt_port_socket.c::nxt_port_read_handler()
However it doesn't seem to be zeroed out or anything and so unset
fields will just contain whatever happens to be in memory.
E.g At start up
2022/06/16 23:19:28 [info] 36687#36687 unit 1.27.0 started
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32621
nxt_port_read_msg_process: msg->cancelled : 0
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 0
nxt_port_read_msg_process: msg->cancelled : 32767
nxt_port_read_msg_process: msg->cancelled : 32767
And I suspect fmsg->cancelled will be similar as it seem to just be
essentially set from msg.
So if we think that ->cancelled is a zombie member and the only place
I see it getting set is
516 msg.cancelled = 0;
in src/nxt_port_rpc.c::nxt_port_rpc_close()
which looks like some clean up function. It isn't called before
nxt_port_read_msg_process(), which follows the above output... and it
doesn't _seem_ to be 0 initialised anywhere else, then it _appears_ we
could just remove the whole of
1202 if (nxt_fast_path(fmsg->cancelled == 0)) {
1203
1204 if (msg->port_msg.mmap) {
1205 nxt_port_mmap_read(task, msg);
1206 }
1207
1208 nxt_buf_chain_add(&fmsg->buf, msg->buf);
1209
1210 fmsg->size += msg->size;
1211 msg->buf = NULL;
1212 b = NULL;
1213
1214 if (nxt_fast_path(msg->port_msg.mf == 0)) {
1215
1216 b = fmsg->buf;
1217
1218 port->handler(task, fmsg);
1219
1220 msg->buf = fmsg->buf;
1221 msg->fd[0] = fmsg->fd[0];
1222 msg->fd[1] = fmsg->fd[1];
1223
1224 /*
1225 * To disable instant completion or buffer re-usage,
1226 * handler should reset 'msg.buf'.
1227 */
1228 if (!msg->port_msg.mmap && msg->buf == b) {
1229 nxt_port_buf_free(port, b);
1230 }
1231 }
1232 }
from src/nxt_port_socket.c::nxt_port_read_msg_process()
How we get to nxt_port_read_msg_process() is fairly straightforward
(gdb) bt
...
#4 0x00000000004103a1 in nxt_port_read_msg_process (task=0x14bdbf0,
port=0x14c8230, msg=0x7ffeafc21c20) at src/nxt_port_socket.c:1177
#5 0x000000000040f529 in nxt_port_read_handler (task=0x14bdbf0,
obj=0x14c8230, data=0x0) at src/nxt_port_socket.c:778
#6 0x000000000041f8cc in nxt_event_engine_start (engine=0x14bdbf0)
at src/nxt_event_engine.c:542
#7 0x000000000040a227 in main (argc=2, argv=0x7ffeafc21ea8)
at src/nxt_main.c:35
Thoughts?
Cheers,
Andrew
More information about the unit
mailing list