From e6ba8d5ede7fb2af9312180459a8a3db0cb081be Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Wed, 10 Mar 2010 12:03:09 +0000 Subject: OTP-8475 status lock needed when looking up ioq from async threads Driver threads, such as async threads, using port data locks peeked at the port status field without proper locking when looking up the driver queue. --- erts/emulator/beam/io.c | 34 ++++++++++++---- .../emulator/test/driver_SUITE_data/ioq_exit_drv.c | 46 +++++++++++++++------- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/erts/emulator/beam/io.c b/erts/emulator/beam/io.c index ad0b909b2a..3309b77086 100644 --- a/erts/emulator/beam/io.c +++ b/erts/emulator/beam/io.c @@ -77,15 +77,35 @@ static ERTS_INLINE ErlIOQueue* drvport2ioq(ErlDrvPort drvport) { int ix = (int) drvport; + Uint32 status; + if (ix < 0 || erts_max_ports <= ix) return NULL; - if (erts_port[ix].status & ERTS_PORT_SFLGS_INVALID_DRIVER_LOOKUP) - return NULL; - ERTS_LC_ASSERT(!erts_port[ix].port_data_lock - || erts_lc_mtx_is_locked(&erts_port[ix].port_data_lock->mtx)); - ERTS_SMP_LC_ASSERT(erts_port[ix].port_data_lock - || erts_lc_is_port_locked(&erts_port[ix])); - return &erts_port[ix].ioq; + + if (erts_get_scheduler_data()) { + ERTS_SMP_LC_ASSERT(erts_lc_is_port_locked(&erts_port[ix])); + ERTS_LC_ASSERT(!erts_port[ix].port_data_lock + || erts_lc_mtx_is_locked( + &erts_port[ix].port_data_lock->mtx)); + + status = erts_port[ix].status; + } + else { + erts_smp_port_state_lock(&erts_port[ix]); + status = erts_port[ix].status; + erts_smp_port_state_unlock(&erts_port[ix]); + + ERTS_LC_ASSERT((status & ERTS_PORT_SFLGS_INVALID_DRIVER_LOOKUP) + || erts_port[ix].port_data_lock); + ERTS_LC_ASSERT(!erts_port[ix].port_data_lock + || erts_lc_mtx_is_locked( + &erts_port[ix].port_data_lock->mtx)); + + } + + return ((status & ERTS_PORT_SFLGS_INVALID_DRIVER_LOOKUP) + ? NULL + : &erts_port[ix].ioq); } static ERTS_INLINE int diff --git a/erts/emulator/test/driver_SUITE_data/ioq_exit_drv.c b/erts/emulator/test/driver_SUITE_data/ioq_exit_drv.c index 2048d06123..c7a42aa687 100644 --- a/erts/emulator/test/driver_SUITE_data/ioq_exit_drv.c +++ b/erts/emulator/test/driver_SUITE_data/ioq_exit_drv.c @@ -1,19 +1,20 @@ -/* ``The contents of this file are subject to the Erlang Public License, +/* + * %CopyrightBegin% + * + * Copyright Ericsson AB 2007-2010. All Rights Reserved. + * + * The contents of this file are subject to the Erlang Public License, * Version 1.1, (the "License"); you may not use this file except in * compliance with the License. You should have received a copy of the * Erlang Public License along with this software. If not, it can be - * retrieved via the world wide web at http://www.erlang.org/. - * + * retrieved online at http://www.erlang.org/. + * * Software distributed under the License is distributed on an "AS IS" * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See * the License for the specific language governing rights and limitations * under the License. - * - * The Initial Developer of the Original Code is Ericsson Utvecklings AB. - * Portions created by Ericsson are Copyright 1999, Ericsson Utvecklings - * AB. All Rights Reserved.'' - * - * $Id$ + * + * %CopyrightEnd% */ /* @@ -77,6 +78,7 @@ typedef struct { int ofd; int outstanding_async_task; long async_task; + ErlDrvPDL pdl; #ifdef HAVE_POLL_H struct erl_drv_event_data event_data; #endif @@ -144,6 +146,7 @@ start(ErlDrvPort port, char *command) ddp->ofd = -1; ddp->outstanding_async_task = 0; ddp->async_task = -1; + ddp->pdl = driver_pdl_create(port); #ifdef HAVE_POLL_H ddp->event_data.events = (short) 0; ddp->event_data.revents = (short) 0; @@ -217,8 +220,9 @@ static int control(ErlDrvData drv_data, res_str = "error: command not supported"; goto done; } - + driver_pdl_lock(ddp->pdl); driver_enq(ddp->port, "!", 1); + driver_pdl_unlock(ddp->pdl); ddp->test = (IOQExitTest) command; res_str = "ok"; @@ -333,8 +337,11 @@ static void ready_input(ErlDrvData drv_data, ErlDrvEvent event) ddp->ifd = -1; if (ddp->test == IOQ_EXIT_READY_INPUT_ASYNC) do_driver_async(ddp); - else + else { + driver_pdl_lock(ddp->pdl); driver_deq(ddp->port, 1); + driver_pdl_unlock(ddp->pdl); + } } #endif } @@ -352,8 +359,11 @@ static void ready_output(ErlDrvData drv_data, ErlDrvEvent event) ddp->ofd = -1; if (ddp->test == IOQ_EXIT_READY_OUTPUT_ASYNC) do_driver_async(ddp); - else + else { + driver_pdl_lock(ddp->pdl); driver_deq(ddp->port, 1); + driver_pdl_unlock(ddp->pdl); + } } #endif } @@ -366,8 +376,11 @@ static void timeout(ErlDrvData drv_data) if (ddp->test == IOQ_EXIT_TIMEOUT_ASYNC) do_driver_async(ddp); - else + else { + driver_pdl_lock(ddp->pdl); driver_deq(ddp->port, 1); + driver_pdl_unlock(ddp->pdl); + } } static void ready_async(ErlDrvData drv_data, ErlDrvThreadData thread_data) @@ -377,7 +390,9 @@ static void ready_async(ErlDrvData drv_data, ErlDrvThreadData thread_data) PRINTF(("ready_async(%p, %p) called\r\n", drv_data, thread_data)); if (drv_data == (ErlDrvData) thread_data) { + driver_pdl_lock(ddp->pdl); driver_deq(ddp->port, 1); + driver_pdl_unlock(ddp->pdl); ddp->outstanding_async_task = 0; } } @@ -397,8 +412,11 @@ static void event(ErlDrvData drv_data, ddp->ofd = -1; if (ddp->test == IOQ_EXIT_EVENT_ASYNC) do_driver_async(ddp); - else + else { + driver_pdl_lock(ddp->pdl); driver_deq(ddp->port, 1); + driver_pdl_unlock(ddp->pdl); + } } #endif } -- cgit v1.2.3