From d35dc5f707f307300665c7da2ae3092d0baa9bdd Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Thu, 22 Apr 2010 12:00:00 +0200 Subject: erts: Patch 1113 OTP-8475 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. OTP-8487 A call to the BIF unregister(RegName) when a port had the name RegName registered in the runtime system without SMP support caused a runtime system crash. (Thanks to Per Hedeland for the bugfix and test case.) OTP-8591 Fix memory management bug causing crash of non-SMP emulator with async threads enabled. The bug did first appear in R13B03. --- erts/emulator/beam/io.c | 34 ++++++++++++---- erts/emulator/beam/register.c | 19 +++++---- erts/emulator/sys/common/erl_mseg.c | 30 ++++++-------- .../emulator/test/driver_SUITE_data/ioq_exit_drv.c | 46 +++++++++++++++------- erts/emulator/test/port_SUITE.erl | 20 ++++++---- 5 files changed, 96 insertions(+), 53 deletions(-) (limited to 'erts/emulator') 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/beam/register.c b/erts/emulator/beam/register.c index 7ba097382a..964c10a380 100644 --- a/erts/emulator/beam/register.c +++ b/erts/emulator/beam/register.c @@ -1,19 +1,19 @@ /* * %CopyrightBegin% - * - * Copyright Ericsson AB 1996-2009. All Rights Reserved. - * + * + * Copyright Ericsson AB 1996-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 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. - * + * * %CopyrightEnd% */ @@ -500,8 +500,8 @@ int erts_unregister_name(Process *c_p, if ((rp = (RegProc*) hash_get(&process_reg, (void*) &r)) != NULL) { if (rp->pt) { -#ifdef ERTS_SMP if (port != rp->pt) { +#ifdef ERTS_SMP if (port) { ERTS_SMP_LC_ASSERT(port != c_prt); erts_smp_port_unlock(port); @@ -519,10 +519,13 @@ int erts_unregister_name(Process *c_p, port = erts_id2port(id, NULL, 0); goto restart; } +#endif port = rp->pt; } -#endif - ERTS_SMP_LC_ASSERT(rp->pt == port && erts_lc_is_port_locked(port)); + + ASSERT(rp->pt == port); + ERTS_SMP_LC_ASSERT(erts_lc_is_port_locked(port)); + rp->pt->reg = NULL; if (IS_TRACED_FL(port, F_TRACE_PORTS)) { diff --git a/erts/emulator/sys/common/erl_mseg.c b/erts/emulator/sys/common/erl_mseg.c index f4e21bc05f..5dfd66bd7c 100644 --- a/erts/emulator/sys/common/erl_mseg.c +++ b/erts/emulator/sys/common/erl_mseg.c @@ -1,19 +1,19 @@ /* * %CopyrightBegin% - * - * Copyright Ericsson AB 2002-2009. All Rights Reserved. - * + * + * Copyright Ericsson AB 2002-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 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. - * + * * %CopyrightEnd% */ @@ -287,13 +287,9 @@ check_schedule_cache_check(void) static void mseg_shutdown(void) { -#ifdef ERTS_SMP erts_mtx_lock(&mseg_mutex); -#endif mseg_clear_cache(); -#ifdef ERTS_SMP erts_mtx_unlock(&mseg_mutex); -#endif } static ERTS_INLINE void * @@ -375,8 +371,9 @@ mseg_recreate(void *old_seg, Uint old_size, Uint new_size) static ERTS_INLINE cache_desc_t * alloc_cd(void) -{ +{ cache_desc_t *cd = free_cache_descs; + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); if (cd) free_cache_descs = cd->next; return cd; @@ -385,6 +382,7 @@ alloc_cd(void) static ERTS_INLINE void free_cd(cache_desc_t *cd) { + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); cd->next = free_cache_descs; free_cache_descs = cd; } @@ -393,6 +391,7 @@ free_cd(cache_desc_t *cd) static ERTS_INLINE void link_cd(cache_desc_t *cd) { + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); if (cache) cache->prev = cd; cd->next = cache; @@ -410,6 +409,7 @@ link_cd(cache_desc_t *cd) static ERTS_INLINE void end_link_cd(cache_desc_t *cd) { + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); if (cache_end) cache_end->next = cd; cd->next = NULL; @@ -427,7 +427,7 @@ end_link_cd(cache_desc_t *cd) static ERTS_INLINE void unlink_cd(cache_desc_t *cd) { - + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); if (cd->next) cd->next->prev = cd->prev; else @@ -445,6 +445,7 @@ static ERTS_INLINE void check_cache_limits(void) { cache_desc_t *cd; + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); max_cached_seg_size = 0; min_cached_seg_size = ~((Uint) 0); for (cd = cache; cd; cd = cd->next) { @@ -463,7 +464,7 @@ adjust_cache_size(int force_check_limits) int check_limits = force_check_limits; Sint max_cached = ((Sint) segments.current.watermark - (Sint) segments.current.no); - + ERTS_LC_ASSERT(erts_lc_mtx_is_locked(&mseg_mutex)); while (((Sint) cache_size) > max_cached && ((Sint) cache_size) > 0) { ASSERT(cache_end); cd = cache_end; @@ -487,9 +488,7 @@ adjust_cache_size(int force_check_limits) static void check_cache(void *unused) { -#ifdef ERTS_SMP erts_mtx_lock(&mseg_mutex); -#endif is_cache_check_scheduled = 0; @@ -502,10 +501,7 @@ check_cache(void *unused) INC_CC(check_cache); -#ifdef ERTS_SMP erts_mtx_unlock(&mseg_mutex); -#endif - } static void 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 } diff --git a/erts/emulator/test/port_SUITE.erl b/erts/emulator/test/port_SUITE.erl index 9a09d20eab..b9100738e4 100644 --- a/erts/emulator/test/port_SUITE.erl +++ b/erts/emulator/test/port_SUITE.erl @@ -1,19 +1,19 @@ %% %% %CopyrightBegin% -%% -%% Copyright Ericsson AB 1997-2009. All Rights Reserved. -%% +%% +%% Copyright Ericsson AB 1997-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 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. -%% +%% %% %CopyrightEnd% %% @@ -88,7 +88,8 @@ otp_3906/1, otp_4389/1, win_massive/1, win_massive_client/1, mix_up_ports/1, otp_5112/1, otp_5119/1, otp_6224/1, exit_status_multi_scheduling_block/1, ports/1, - spawn_driver/1,spawn_executable/1]). + spawn_driver/1,spawn_executable/1, + unregister_name/1]). -export([]). @@ -112,7 +113,8 @@ all(suite) -> otp_3906, otp_4389, win_massive, mix_up_ports, otp_5112, otp_5119, exit_status_multi_scheduling_block, - ports, spawn_driver, spawn_executable + ports, spawn_driver, spawn_executable, + unregister_name ]. -define(DEFAULT_TIMEOUT, ?t:minutes(5)). @@ -1434,6 +1436,10 @@ spawn_executable(Config) when is_list(Config) -> ?line test_server:timetrap_cancel(Dog), ok. +unregister_name(Config) when is_list(Config) -> + ?line true = register(crash, open_port({spawn, "sleep 100"}, [])), + ?line true = unregister(crash). + test_bat_file(Dir) -> FN = "tf.bat", Full = filename:join([Dir,FN]), -- cgit v1.2.3