aboutsummaryrefslogtreecommitdiffstats
path: root/erts
diff options
context:
space:
mode:
authorJohn Högberg <[email protected]>2018-06-27 14:40:18 +0200
committerJohn Högberg <[email protected]>2018-06-28 08:39:15 +0200
commita9e579341181ed877379de022a261da81fc7cf8c (patch)
treea2688aa2f489121e4ff7a02469e274af9021866e /erts
parentda06fd040775fffee17409ebbd6fa797e34d6f99 (diff)
downloadotp-a9e579341181ed877379de022a261da81fc7cf8c.tar.gz
otp-a9e579341181ed877379de022a261da81fc7cf8c.tar.bz2
otp-a9e579341181ed877379de022a261da81fc7cf8c.zip
Fix a race condition when generating async operation ids
The counter used for generating async operation ids was a plain int shared between all ports, which was incorrect but mostly worked fine since the ids only had to be unique on a per-port basis. However, some compilers (notably GCC 8.1.1) generated code that assumed that this value didn't change between reads. Using a shortened version of enq_async_w_tmo as an example: int id = async_ref++; op->id = id; //A return id; //B In GCC 7 and earlier, `async_ref` would be read once and assigned to `id` before being incremented, which kept the values at A and B consistent. In GCC 8, `async_ref` was read when assigned at A and read again at B, and then incremented, which made them inconsistent if we raced with another port. This commit fixes the issue by removing `async_ref` altogether and replacing it with a per-port counter which makes it impossible to race with someone else.
Diffstat (limited to 'erts')
-rw-r--r--erts/emulator/drivers/common/inet_drv.c9
1 files changed, 5 insertions, 4 deletions
diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c
index ebd13e6f05..3029958f26 100644
--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -1025,6 +1025,7 @@ typedef struct {
inet_async_op* oph; /* queue head or NULL */
inet_async_op* opt; /* queue tail or NULL */
inet_async_op op_queue[INET_MAX_ASYNC]; /* call queue */
+ int op_ref; /* queue reference generator */
int active; /* 0 = passive, 1 = active, 2 = active once */
Sint16 active_count; /* counter for {active,N} */
@@ -1273,8 +1274,7 @@ static int packet_inet_output(udp_descriptor* udesc, HANDLE event);
/* convert descriptor pointer to inet_descriptor pointer */
#define INETP(d) (&(d)->inet)
-static int async_ref = 0; /* async reference id generator */
-#define NEW_ASYNC_ID() ((async_ref++) & 0xffff)
+#define NEW_ASYNC_ID(desc) ((desc)->op_ref++ & 0xffff)
/* check for transition from active to passive */
#define INET_CHECK_ACTIVE_TO_PASSIVE(inet) \
@@ -1925,7 +1925,7 @@ static void enq_multi_op(tcp_descriptor *desc, char *buf, int req,
ErlDrvTermData caller, MultiTimerData *timeout,
ErlDrvMonitor *monitorp)
{
- int id = NEW_ASYNC_ID();
+ int id = NEW_ASYNC_ID(INETP(desc));
enq_old_multi_op(desc,id,req,caller,timeout,monitorp);
if (buf != NULL)
put_int16(id, buf);
@@ -1994,7 +1994,7 @@ static int remove_multi_op(tcp_descriptor *desc, int *id_p, int *req_p,
static int enq_async_w_tmo(inet_descriptor* desc, char* buf, int req, unsigned timeout,
ErlDrvMonitor *monitorp)
{
- int id = NEW_ASYNC_ID();
+ int id = NEW_ASYNC_ID(desc);
inet_async_op* opp;
if ((opp = desc->oph) == NULL) /* queue empty */
@@ -8310,6 +8310,7 @@ static ErlDrvData inet_start(ErlDrvPort port, int size, int protocol)
desc->delimiter = '\n'; /* line delimiting char */
desc->oph = NULL;
desc->opt = NULL;
+ desc->op_ref = 0;
desc->peer_ptr = NULL;
desc->name_ptr = NULL;