Skip to content

Commit 4b5f9b7

Browse files
committed
[erts] Make sure nosuspend command signals to ports are handled properly
No-suspend port command signals (i.e. port command signals sent using the `erlang:port_command/3` BIF or the `erlang:send/3` BIF with the `nosuspend` option) were not aborted properly in all scenarios which could leave the port queue in a busy state indefinitely. Asynchronously sent no-suspend command signals (i.e, port command signals sent using the `erlang:send/3` BIF with the `nosuspend` option) could sometimes be delivered even though the port was busy.
1 parent c388a2d commit 4b5f9b7

10 files changed

Lines changed: 219 additions & 22 deletions

File tree

erts/emulator/beam/dist.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,8 @@ int erts_do_net_exits(DistEntry *dep, Eterm reason)
10241024
ERTS_LC_ASSERT(erts_lc_is_port_locked(erts_port_lookup_raw(dep->cid)));
10251025

10261026
if (erts_port_task_is_scheduled(&dep->dist_cmd))
1027-
erts_port_task_abort(&dep->dist_cmd);
1027+
erts_port_task_abort(erts_port_lookup_raw(dep->cid),
1028+
&dep->dist_cmd);
10281029
}
10291030
else {
10301031
ASSERT(is_internal_pid(dep->cid));

erts/emulator/beam/erl_hl_timer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2757,7 +2757,7 @@ erts_cancel_port_timer(Port *c_prt)
27572757
if (tval == ERTS_PTMR_TIMEDOUT) {
27582758
while (!erts_port_task_is_scheduled(&c_prt->timeout_task))
27592759
erts_thr_yield();
2760-
erts_port_task_abort(&c_prt->timeout_task);
2760+
erts_port_task_abort(c_prt, &c_prt->timeout_task);
27612761
erts_atomic_set_nob(&c_prt->common.timer, ERTS_PTMR_NONE);
27622762
return;
27632763
}

erts/emulator/beam/erl_port.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,7 @@ enum {
871871
#define ERTS_P2P_SIG_DATA_FLG_BROKEN_LINK ERTS_P2P_SIG_DATA_FLG(5)
872872
#define ERTS_P2P_SIG_DATA_FLG_SCHED ERTS_P2P_SIG_DATA_FLG(6)
873873
#define ERTS_P2P_SIG_DATA_FLG_ASYNC ERTS_P2P_SIG_DATA_FLG(7)
874+
#define ERTS_P2P_SIG_DATA_FLG_ASYNC_NOSUSPEND ERTS_P2P_SIG_DATA_FLG(8)
874875

875876
struct ErtsProc2PortSigData_ {
876877
int flags;

erts/emulator/beam/erl_port_task.c

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,8 @@ enqueue_task(Port *pp,
10011001
if (ns_pthlp) {
10021002
ns_pthlp->u.next = pp->sched.taskq.local.busy.nosuspend;
10031003
pp->sched.taskq.local.busy.nosuspend = ns_pthlp;
1004+
erts_atomic32_read_bor_nob(&pp->sched.flags,
1005+
ERTS_PTS_FLG_HAVE_NS_TASKS);
10041006
}
10051007
if (pp->sched.taskq.in.last) {
10061008
ASSERT(pp->sched.taskq.in.first);
@@ -1298,7 +1300,7 @@ erts_port_task_tmp_handle_detach(ErtsPortTaskHandle *pthp)
12981300
*/
12991301

13001302
int
1301-
erts_port_task_abort(ErtsPortTaskHandle *pthp)
1303+
erts_port_task_abort(Port *pp, ErtsPortTaskHandle *pthp)
13021304
{
13031305
int res;
13041306
ErtsPortTask *ptp;
@@ -1326,22 +1328,33 @@ erts_port_task_abort(ErtsPortTaskHandle *pthp)
13261328
res = - 1; /* Task already aborted, executing, or executed */
13271329
else {
13281330
reset_port_task_handle(pthp);
1331+
switch (ptp->type) {
1332+
case ERTS_PORT_TASK_PROC_SIG: {
1333+
int abort_type = ((ptp->u.alive.flags
1334+
& (ERTS_PT_FLG_NOSUSPEND
1335+
| ERTS_PT_FLG_ASYNC_NOSUSPEND))
1336+
? ERTS_PROC2PORT_SIG_ABORT_NOSUSPEND
1337+
: ERTS_PROC2PORT_SIG_ABORT);
1338+
ASSERT(pp);
1339+
abort_signal_task(pp, abort_type, ptp->type, &ptp->u.alive.td,
1340+
pp->sched.taskq.bpq != NULL);
1341+
break;
1342+
}
1343+
case ERTS_PORT_TASK_INPUT:
1344+
case ERTS_PORT_TASK_OUTPUT:
13291345
#if ERTS_POLL_USE_SCHEDULER_POLLING
1330-
if (erts_sched_poll_enabled()) {
1331-
switch (ptp->type) {
1332-
case ERTS_PORT_TASK_INPUT:
1333-
case ERTS_PORT_TASK_OUTPUT:
1346+
if (erts_sched_poll_enabled()) {
13341347
if (ptp->u.alive.td.io.is_scheduler_event) {
13351348
ASSERT(erts_atomic_read_nob(
13361349
&erts_port_task_outstanding_io_tasks) > 0);
13371350
erts_atomic_dec_relb(&erts_port_task_outstanding_io_tasks);
13381351
}
1339-
break;
1340-
default:
1341-
break;
13421352
}
1343-
}
13441353
#endif
1354+
break;
1355+
default:
1356+
break;
1357+
}
13451358
res = 0;
13461359
}
13471360
}
@@ -1369,7 +1382,6 @@ erts_port_task_abort_nosuspend_tasks(Port *pp)
13691382
ErtsPortTaskHandle *saved_pthp;
13701383
#endif
13711384
ErtsPortTaskType type;
1372-
ErtsPortTaskTypeData td;
13731385
ErtsPortTaskHandle *pthp;
13741386
ErtsPortTask *ptp;
13751387
ErtsPortTaskHandleList *pthlp;
@@ -1413,13 +1425,14 @@ erts_port_task_abort_nosuspend_tasks(Port *pp)
14131425
reset_port_task_handle(pthp);
14141426

14151427
type = ptp->type;
1416-
td = ptp->u.alive.td;
1428+
1429+
abort_nosuspend_task(pp, type, &ptp->u.alive.td,
1430+
pp->sched.taskq.bpq != NULL);
14171431

14181432
if (dhndl != ERTS_THR_PRGR_DHANDLE_MANAGED)
14191433
erts_thr_progress_unmanaged_continue(dhndl);
14201434
schedule_port_task_handle_list_free(pthlp);
14211435

1422-
abort_nosuspend_task(pp, type, &td, pp->sched.taskq.bpq != NULL);
14231436
}
14241437
}
14251438

@@ -1520,8 +1533,6 @@ erts_port_task_schedule(Eterm id,
15201533
}
15211534

15221535
add_flags = ERTS_PTS_FLG_HAVE_TASKS;
1523-
if (ns_pthlp)
1524-
add_flags |= ERTS_PTS_FLG_HAVE_NS_TASKS;
15251536

15261537
prof_runnable_ports = erts_system_profile_flags.runnable_ports;
15271538
if (prof_runnable_ports)

erts/emulator/beam/erl_port_task.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ typedef erts_atomic_t ErtsPortTaskHandle;
5454
#define ERTS_PT_FLG_NOSUSPEND (1 << 2)
5555
#define ERTS_PT_FLG_REF (1 << 3)
5656
#define ERTS_PT_FLG_BAD_OUTPUT (1 << 4)
57+
#define ERTS_PT_FLG_ASYNC_NOSUSPEND (1 << 5)
5758

5859
typedef enum {
5960
ERTS_PORT_TASK_INPUT = 0,
@@ -239,7 +240,7 @@ void erts_port_task_init(void);
239240
void erts_port_task_pre_alloc_init_thread(void);
240241

241242
void erts_port_task_tmp_handle_detach(ErtsPortTaskHandle *);
242-
int erts_port_task_abort(ErtsPortTaskHandle *);
243+
int erts_port_task_abort(Port *, ErtsPortTaskHandle *);
243244

244245
void erts_port_task_abort_nosuspend_tasks(Port *);
245246

erts/emulator/beam/io.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,8 @@ erts_port_output(Process *c_p,
21102110
ASSERT(esdp);
21112111
ns_pthp = &esdp->nosuspend_port_task_handle;
21122112
sigdp->flags &= ~ERTS_P2P_SIG_DATA_FLG_NOSUSPEND;
2113+
sigdp->flags |= ERTS_P2P_SIG_DATA_FLG_ASYNC_NOSUSPEND;
2114+
task_flags = ERTS_PT_FLG_WAIT_BUSY|ERTS_PT_FLG_ASYNC_NOSUSPEND;
21132115
}
21142116
else if (flags & ERTS_P2P_SIG_DATA_FLG_NOSUSPEND)
21152117
task_flags = ERTS_PT_FLG_NOSUSPEND;
@@ -2141,7 +2143,7 @@ erts_port_output(Process *c_p,
21412143
if (!async_nosuspend)
21422144
return ERTS_PORT_OP_BUSY_SCHEDULED;
21432145
else {
2144-
if (erts_port_task_abort(ns_pthp) == 0)
2146+
if (erts_port_task_abort(prt, ns_pthp) == 0)
21452147
return ERTS_PORT_OP_BUSY;
21462148
else
21472149
erts_port_task_tmp_handle_detach(ns_pthp);

erts/emulator/sys/common/erl_check_io.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ static ERTS_INLINE void
573573
abort_task(Eterm id, ErtsPortTaskHandle *pthp, EventStateType type)
574574
{
575575
if (is_not_nil(id) && erts_port_task_is_scheduled(pthp)) {
576-
erts_port_task_abort(pthp);
576+
erts_port_task_abort(NULL, pthp);
577577
ASSERT(erts_is_port_alive(id));
578578
}
579579
}

erts/emulator/test/busy_port_SUITE.erl

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@
2828
scheduling_delay_busy/1,
2929
scheduling_delay_busy_nosuspend/1,
3030
scheduling_busy_link/1,
31-
busy_with_signals/1]).
31+
busy_with_signals/1,
32+
async_par_nosuspend_abort_cleanup/1,
33+
async_nopar_nosuspend_abort_cleanup/1,
34+
sync_par_nosuspend_abort_cleanup/1,
35+
sync_nopar_nosuspend_abort_cleanup/1]).
3236

3337
-include_lib("common_test/include/ct.hrl").
3438

@@ -44,7 +48,11 @@ all() ->
4448
no_trap_exit, no_trap_exit_unlinked, trap_exit,
4549
multiple_writers, hard_busy_driver, soft_busy_driver,
4650
scheduling_delay_busy,scheduling_delay_busy_nosuspend,
47-
scheduling_busy_link, busy_with_signals].
51+
scheduling_busy_link, busy_with_signals,
52+
async_par_nosuspend_abort_cleanup,
53+
async_nopar_nosuspend_abort_cleanup,
54+
sync_par_nosuspend_abort_cleanup,
55+
sync_nopar_nosuspend_abort_cleanup].
4856

4957
init_per_testcase(_Case, Config) when is_list(Config) ->
5058
Killer = spawn(fun() -> killer_loop([]) end),
@@ -844,6 +852,69 @@ flood_with_exit_signals(Pid, N) ->
844852
exit(Pid, pling),
845853
flood_with_exit_signals(Pid, N-1).
846854

855+
async_par_nosuspend_abort_cleanup(Config) when is_list(Config) ->
856+
nosuspend_abort_cleanup_tests(Config, false, true).
857+
858+
async_nopar_nosuspend_abort_cleanup(Config) when is_list(Config) ->
859+
nosuspend_abort_cleanup_tests(Config, false, false).
860+
861+
sync_par_nosuspend_abort_cleanup(Config) when is_list(Config) ->
862+
nosuspend_abort_cleanup_tests(Config, true, true).
863+
864+
sync_nopar_nosuspend_abort_cleanup(Config) when is_list(Config) ->
865+
nosuspend_abort_cleanup_tests(Config, true, false).
866+
867+
nosuspend_abort_cleanup_tests(Config, Sync, Parallelism) ->
868+
Master = self(),
869+
process_flag(priority,high),
870+
Drv = "nosuspend_test_drv",
871+
DataDir = proplists:get_value(data_dir, Config),
872+
erl_ddll:start(),
873+
case erl_ddll:load_driver(DataDir, Drv) of
874+
ok ->
875+
ok;
876+
{error, Error} ->
877+
ct:fail(erl_ddll:format_error(Error))
878+
end,
879+
Port = open_port({spawn_driver, Drv},
880+
[{busy_limits_msgq, {1024, 2048}},
881+
{parallelism, Parallelism}]),
882+
NoSuspFun = if Sync ->
883+
fun () ->
884+
port_command(Port, <<0:(16*8*1024)>>, [nosuspend])
885+
end;
886+
true ->
887+
fun () ->
888+
erlang:send(Port,
889+
{Master, {command, <<0:(16*8*1024)>>}},
890+
[nosuspend])
891+
end
892+
end,
893+
NoSuspers = lists:map(fun (_) -> spawn_link(NoSuspFun) end,
894+
lists:seq(1, 10000)),
895+
_= port_control(Port, $B, <<>>),
896+
receive after 1000 -> ok end,
897+
_= port_control(Port, $N, <<>>),
898+
SyncCmd = spawn_link(fun () ->
899+
port_command(Port, <<"should not block for long">>),
900+
Master ! {self(), done}
901+
end),
902+
receive
903+
{SyncCmd, done} -> ok
904+
after
905+
10000 -> ct:fail(port_command_blocked)
906+
end,
907+
lists:foreach(fun (P) ->
908+
unlink(P),
909+
exit(P,kill),
910+
false = is_process_alive(P)
911+
end,
912+
[SyncCmd | NoSuspers]),
913+
true = erlang:port_close(Port),
914+
ok = erl_ddll:unload_driver(Drv),
915+
ok = erl_ddll:stop(),
916+
ok.
917+
847918
%%% Utilities.
848919

849920
pal(_F,_A) -> ok.

erts/emulator/test/busy_port_SUITE_data/Makefile.src

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
# %CopyrightEnd%
1919
#
2020

21-
all: busy_drv@dll@ hard_busy_drv@dll@ soft_busy_drv@dll@ scheduling_drv@dll@
21+
all: busy_drv@dll@ hard_busy_drv@dll@ soft_busy_drv@dll@ scheduling_drv@dll@ nosuspend_test_drv@dll@
2222

2323
@SHLIB_RULES@
2424

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* %CopyrightBegin%
3+
*
4+
* SPDX-License-Identifier: Apache-2.0
5+
*
6+
* Copyright Ericsson AB 2026. All Rights Reserved.
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*
20+
* %CopyrightEnd%
21+
*/
22+
23+
#include <errno.h>
24+
#include <stdbool.h>
25+
#include "erl_driver.h"
26+
27+
static void stop(ErlDrvData drv_data);
28+
static ErlDrvData start(ErlDrvPort port, char *command);
29+
static void output(ErlDrvData drv_data, char *buf, ErlDrvSizeT len);
30+
static ErlDrvSSizeT control(ErlDrvData drv_data, unsigned int command, char *buf,
31+
ErlDrvSizeT len, char **rbuf, ErlDrvSizeT rlen);
32+
33+
static ErlDrvEntry nosuspend_test_drv_entry = {
34+
NULL /* init */,
35+
start,
36+
stop,
37+
output,
38+
NULL /* ready_input */,
39+
NULL /* ready_output */,
40+
"nosuspend_test_drv",
41+
NULL /* finish */,
42+
NULL /* handle */,
43+
control,
44+
NULL /* timeout */,
45+
NULL /* outputv */,
46+
NULL /* ready_async */,
47+
NULL /* flush */,
48+
NULL /* call */,
49+
NULL /* event */,
50+
ERL_DRV_EXTENDED_MARKER,
51+
ERL_DRV_EXTENDED_MAJOR_VERSION,
52+
ERL_DRV_EXTENDED_MINOR_VERSION,
53+
ERL_DRV_FLAG_USE_PORT_LOCKING,
54+
NULL /* handle2 */,
55+
NULL /* handle_monitor */,
56+
NULL /* stop_select */
57+
};
58+
59+
typedef struct {
60+
ErlDrvPort port;
61+
bool is_busy;
62+
} PortState;
63+
64+
DRIVER_INIT(nosuspend_test_drv)
65+
{
66+
return &nosuspend_test_drv_entry;
67+
}
68+
69+
static void stop(ErlDrvData drv_data)
70+
{
71+
driver_free(drv_data);
72+
}
73+
74+
static ErlDrvData start(ErlDrvPort port, char *command)
75+
{
76+
PortState *state = driver_alloc(sizeof(PortState));
77+
if (!state)
78+
return ERL_DRV_ERROR_GENERAL;
79+
state->port = port;
80+
state->is_busy = false;
81+
return (ErlDrvData) state;
82+
}
83+
84+
static void output(ErlDrvData drv_data, char *buf, ErlDrvSizeT len)
85+
{
86+
PortState *state = (PortState *) drv_data;
87+
if (state->is_busy) {
88+
driver_failure_atom(state->port, "got_data_when_busy");
89+
}
90+
}
91+
92+
static ErlDrvSSizeT control(ErlDrvData drv_data, unsigned int command, char *buf,
93+
ErlDrvSizeT len, char **rbuf, ErlDrvSizeT rlen)
94+
{
95+
PortState *state = (PortState *) drv_data;
96+
switch (command) {
97+
case 'B': /* busy */
98+
set_busy_port(state->port, !0);
99+
state->is_busy = true;
100+
break;
101+
case 'N': /* not busy */
102+
set_busy_port(state->port, 0);
103+
state->is_busy = false;
104+
break;
105+
default:
106+
driver_failure_posix(state->port, EINVAL);
107+
break;
108+
}
109+
return 0;
110+
}

0 commit comments

Comments
 (0)