Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Ledest:erlang:24
erlang
0545-erts-Robustify-yielding-ets-insert-2-and-e...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0545-erts-Robustify-yielding-ets-insert-2-and-ets-insert_.patch of Package erlang
From 9b5f5a67557ebd0583a684fe544a42e0d081df43 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson <sverker@erlang.org> Date: Wed, 24 Aug 2022 12:35:48 +0200 Subject: [PATCH 5/5] erts: Robustify yielding ets:insert/2 and ets:insert_new/2 Symptom: VM crash if table is deleted or renamed during ets:insert and ets:insert_new with long list of tuples. * Handle table deletion during yielding insert correctly. Both deletion by ets:delete/1 and owner process exit. We no longer commit the entire list for insert. If table is deleted, the remaining list with uninserted tuples will be freed. * Handle table renaming correctly. Do not change table midstream. Only use initial BIF_ARG_1 for first lookup. Then use 'btid' to identify the table. If table is renamed before we go DB_BUSY (and force others to help) the insert will fail. Otherwise it could be observed that the insert op happened after the table was renamed. * Make sure a successful assisted insert op returns 'true' even if table was deleted by someone else before the initial caller returns. * Make sure BIF arguments are restored if ets:insert fails after trapping. --- erts/emulator/beam/erl_db.c | 455 ++++++++++++++++++------------- erts/emulator/beam/erl_db_util.h | 6 +- lib/stdlib/test/ets_SUITE.erl | 193 ++++++++++++- 3 files changed, 453 insertions(+), 201 deletions(-) diff --git a/erts/emulator/beam/erl_db.c b/erts/emulator/beam/erl_db.c index a32c9803e7..e953381567 100644 --- a/erts/emulator/beam/erl_db.c +++ b/erts/emulator/beam/erl_db.c @@ -241,9 +241,16 @@ static void table_dec_refc(DbTable *tb, erts_aint_t min_val) schedule_free_dbtable(tb); } +static ERTS_INLINE DbTable* btid2tab(Binary* btid) +{ + erts_atomic_t *tbref = erts_binary_to_magic_indirection(btid); + return (DbTable *) erts_atomic_read_nob(tbref); +} + static int -db_table_tid_destructor(Binary *unused) +db_table_tid_destructor(Binary *btid) { + ASSERT(btid2tab(btid) == NULL); return 1; } @@ -262,12 +269,6 @@ make_btid(DbTable *tb) erts_refc_inc(&btid->intern.refc, 1); } -static ERTS_INLINE DbTable* btid2tab(Binary* btid) -{ - erts_atomic_t *tbref = erts_binary_to_magic_indirection(btid); - return (DbTable *) erts_atomic_read_nob(tbref); -} - static DbTable * tid2tab(Eterm tid, Eterm *error_info_p) { @@ -425,6 +426,8 @@ static BIF_RETTYPE ets_select1(Process* p, int bif_ix, Eterm arg1); static BIF_RETTYPE ets_select2(Process* p, DbTable*, Eterm tid, Eterm ms); static BIF_RETTYPE ets_select3(Process* p, DbTable*, Eterm tid, Eterm ms, Sint chunk_size); +static BIF_RETTYPE ets_insert_2_list_continuation(Process* p, + struct ets_insert_2_list_info* ctx); /* * Exported global @@ -457,8 +460,8 @@ free_dbtable(void *vtb) erts_mtx_destroy(&tb->common.fixlock); } - if (tb->common.btid) - erts_bin_release(tb->common.btid); + ASSERT(tb->common.btid); + erts_bin_release(tb->common.btid); erts_flxctr_destroy(&tb->common.counters, ERTS_ALC_T_ETS_CTRS); erts_free(ERTS_ALC_T_DB_TABLE, tb); @@ -466,10 +469,7 @@ free_dbtable(void *vtb) static void schedule_free_dbtable(DbTable* tb) { - /* - * NON-SMP case: Caller is *not* allowed to access the *tb - * structure after this function has returned! - * SMP case: Caller is allowed to access the *common* part of the *tb + /* SMP case: Caller is allowed to access the *common* part of the *tb * structure until the bif has returned (we typically need to * unlock the table lock after this function has returned). * Caller is *not* allowed to access the specialized part @@ -721,24 +721,12 @@ static DbTable* handle_lacking_permission(Process* p, DbTable* tb, Uint* freason_p) { if (tb->common.status & DB_BUSY) { - void* continuation_state; if (!db_is_exclusive(tb, kind)) { db_unlock(tb, kind); db_lock(tb, LCK_WRITE); } - continuation_state = (void*)erts_atomic_read_nob(&tb->common.continuation_state); - if (continuation_state != NULL) { - const long iterations_per_red = 10; - const long reds = iterations_per_red * ERTS_BIF_REDS_LEFT(p); - long nr_of_reductions = DBG_RANDOM_REDS(reds, (Uint)freason_p); - const long init_reds = nr_of_reductions; - tb->common.continuation(&nr_of_reductions, - &continuation_state, - NULL); - if (continuation_state == NULL) { - erts_atomic_set_relb(&tb->common.continuation_state, (Sint)NULL); - } - BUMP_REDS(p, (init_reds - nr_of_reductions) / iterations_per_red); + if (tb->common.continuation_ctx) { + ets_insert_2_list_continuation(p, tb->common.continuation_ctx); } else { delete_all_objects_continue(p, tb); } @@ -868,15 +856,31 @@ DbTable* db_get_table(Process *p, return db_get_table_aux(p, id, what, kind, 0, freason_p); } -static BIF_RETTYPE db_get_table_or_fail_return(DbTable **tb, /* out */ - Eterm table_id, - Uint32 what, - db_lock_kind_t kind, - Uint bif_ix, - Process* p) +static DbTable* db_get_table_or_fail_return(Binary* btid, + Uint32 what, + db_lock_kind_t kind, + Uint bif_ix, + Process* p) { - DB_GET_TABLE(*tb, table_id, what, kind, bif_ix, NULL, p); - return THE_NON_VALUE; + DbTable* tb = btid2tab(btid); + if (!tb) { + p->freason = BADARG | EXF_HAS_EXT_INFO; + p->fvalue = EXI_ID; + } + else { + /* The lock has to be taken to complete the operation */ + db_lock(tb, LCK_WRITE); + if (!(tb->common.status & what)) { + Uint freason; + tb = handle_lacking_permission(p, tb, kind, &freason); + if (!tb) { + BIF_RETTYPE ret = db_bif_fail(p, freason, bif_ix, NULL); + ASSERT(ret == THE_NON_VALUE); (void)ret; + } + } + } + + return tb; } static int insert_named_tab(Eterm name_atom, DbTable* tb, int have_lock) @@ -1508,20 +1512,38 @@ BIF_RETTYPE ets_update_counter_4(BIF_ALIST_4) } typedef enum { + /* + * Prepare phase. Done only by the process calling ets:insert/insert_new. + * All tuples to insert are allocated and copied without table lock. + */ ETS_INSERT_2_LIST_PROCESS_LOCAL, - ETS_INSERT_2_LIST_FAILED_TO_GET_LOCK, - ETS_INSERT_2_LIST_FAILED_TO_GET_LOCK_DESTROY, + + /* + * Commit phase. May be assisted by other calling processes. + * Prepared tuples inserted with table lock. + */ ETS_INSERT_2_LIST_GLOBAL } ets_insert_2_list_status; -typedef struct { +typedef struct ets_insert_2_list_info { ets_insert_2_list_status status; - BIF_RETTYPE destroy_return_value; - DbTable* tb; + Binary* btid; /* identifies the table between traps */ + Eterm tid; /* arg1, also used to detect table name change */ + Eterm list; /* arg2 */ + DbTable* tb; /* cached tb, does not keep table alive between traps */ void* continuation_state; - Binary* continuation_res_bin; + erts_atomic_t return_value; /* Eterm: 'true', 'false' or THE_NON_VALUE */ } ets_insert_2_list_info; +static void cancel_trap_continuation(DbTable* tb) +{ + ets_insert_2_list_info* ctx = tb->common.continuation_ctx; + + if (ctx) { + tb->common.continuation_ctx = NULL; + erts_bin_release(&(ERTS_MAGIC_BIN_FROM_DATA(ctx)->binary)); + } +} static ERTS_INLINE BIF_RETTYPE ets_cret_to_return_value(Process* p, int cret) @@ -1685,63 +1707,77 @@ static int ets_insert_new_2_dbterm_list_has_member(DbTable* tb, void* db_term_li } static void ets_insert_2_list_insert_db_term_list(DbTable* tb, - void* list) + void* db_term_list) { - void* lst = list; + void* tail = db_term_list; void* term = NULL; DbTableMethod* meth = tb->common.meth; + int compress = tb->common.compress; do { LOCAL_VARIABLE(SWord, consumed_reds); consumed_reds = 1; - term = meth->db_dbterm_list_remove_first(&lst); + term = meth->db_dbterm_list_remove_first(&tail); meth->db_put_dbterm(tb, term, 0, &consumed_reds); YCF_CONSUME_REDS(consumed_reds); - } while (lst != NULL); + } while (tail != NULL); return; + + YCF_SPECIAL_CODE_START(ON_DESTROY_STATE); { + ets_insert_2_list_destroy_copied_dbterms(meth, + compress, + tail); + } YCF_SPECIAL_CODE_END(); + } -static void ets_insert_2_list_lock_tbl(Eterm table_id, - Process* p, - Uint bif_ix, - ets_insert_2_list_status on_success_status) +static int ets_insert_2_list_lock_tbl(Binary *btid, + Process* p, + Uint bif_ix, + ets_insert_2_list_status on_success_status) { - BIF_RETTYPE fail_ret; DbTable* tb; + do { - fail_ret = db_get_table_or_fail_return(&tb, - table_id, - DB_WRITE, - LCK_WRITE, - bif_ix, - p); + LOCAL_VARIABLE(ets_insert_2_list_info*,ctx); + ctx = YCF_GET_EXTRA_CONTEXT(); + ASSERT(ctx->status != ETS_INSERT_2_LIST_GLOBAL); + + tb = db_get_table_or_fail_return(btid, + DB_WRITE, + LCK_WRITE, + bif_ix, + p); + ASSERT(ctx->status != ETS_INSERT_2_LIST_GLOBAL); + if (tb == NULL) { - ets_insert_2_list_info *ctx = YCF_GET_EXTRA_CONTEXT(); if (p->freason == TRAP) { - ctx->status = ETS_INSERT_2_LIST_FAILED_TO_GET_LOCK; + YCF_YIELD(); } else { - ctx->status = ETS_INSERT_2_LIST_FAILED_TO_GET_LOCK_DESTROY; - ctx->destroy_return_value = fail_ret; + return 0; } -#ifdef DEBUG + } + else if (is_atom(ctx->tid) && tb->common.the_name != ctx->tid) { /* - * Setting ctx to NULL to avoid that YCF crashes with a - * pointer to stack error when running a debug - * build. YCF_GET_EXTRA_CONTEXT() may change between - * yields as we use stack allocated data for the context - * before the first yield so it is important that the - * context is obtained again with YCF_GET_EXTRA_CONTEXT() - * if a yield might have happened. + * The table has been renamed. We have to fail. This named + * insert op might otherwise be observable as if it happened + * AFTER the table was renamed. + * + * Note that we are allowed to fail here as long as there is no + * atomic name change op from one table to another. */ - ctx = NULL; -#endif - YCF_YIELD(); - } else { - ets_insert_2_list_info *ctx = YCF_GET_EXTRA_CONTEXT(); + p->freason = BADARG | EXF_HAS_EXT_INFO; + p->fvalue = EXI_ID; + db_unlock(tb, LCK_WRITE); + return 0; + } + else { ctx->status = on_success_status; - ASSERT(DB_LOCK_FREE(tb) || erts_lc_rwmtx_is_rwlocked(&tb->common.rwlock)); - ASSERT(!(tb->common.status & DB_DELETE)); } } while (tb == NULL); + + ERTS_LC_ASSERT(DB_LOCK_FREE(tb) || erts_lc_rwmtx_is_rwlocked(&tb->common.rwlock)); + ASSERT(!(tb->common.status & (DB_DELETE|DB_BUSY))); + return 1; } #endif /* YCF_FUNCTIONS */ @@ -1762,13 +1798,14 @@ static ERTS_INLINE int can_insert_without_yield(Uint32 tb_type, #ifdef YCF_FUNCTIONS static BIF_RETTYPE ets_insert_2_list(Process* p, Eterm table_id, + Binary* btid, DbTable *tb, Eterm list, int is_insert_new) { int cret = DB_ERROR_NONE; - void* db_term_list = NULL; /* OBS: memory managements depends on that - db_term_list is initialized to NULL */ + void* db_term_list = NULL; + void* destroy_list = NULL; DbTableMethod* meth = tb->common.meth; int compressed = tb->common.compress; int keypos = tb->common.keypos; @@ -1786,16 +1823,18 @@ static BIF_RETTYPE ets_insert_2_list(Process* p, * table. This is necessary to ensure that the correct reason * for the failure will be available in stack backtrace. */ - ets_insert_2_list_lock_tbl(table_id, p, bif_ix, ETS_INSERT_2_LIST_PROCESS_LOCAL); - db_unlock(tb, LCK_WRITE); - ERTS_BIF_PREP_ERROR_TRAPPED2(ret, p, BADARG, BIF_TRAP_EXPORT(bif_ix), table_id, list); - return ret; + if (ets_insert_2_list_lock_tbl(btid, p, bif_ix, ETS_INSERT_2_LIST_PROCESS_LOCAL)) { + db_unlock(tb, LCK_WRITE); + p->freason = BADARG; + } + return THE_NON_VALUE; } if (can_insert_without_yield(tb_type, list_len, YCF_NR_OF_REDS_LEFT())) { long reds_boost; /* There is enough reductions left to do the inserts directly from the heap without yielding */ - ets_insert_2_list_lock_tbl(table_id, p, bif_ix, ETS_INSERT_2_LIST_PROCESS_LOCAL); + if (!ets_insert_2_list_lock_tbl(btid, p, bif_ix, ETS_INSERT_2_LIST_PROCESS_LOCAL)) + return THE_NON_VALUE; /* Ensure that we will not yield while inserting from heap */ reds_boost = YCF_MAX_NR_OF_REDS - YCF_NR_OF_REDS_LEFT(); YCF_SET_NR_OF_REDS_LEFT(YCF_MAX_NR_OF_REDS); @@ -1815,44 +1854,46 @@ static BIF_RETTYPE ets_insert_2_list(Process* p, /* Copy term list from heap so that other processes can help */ db_term_list = ets_insert_2_list_copy_term_list(meth, compressed, keypos, list); + destroy_list = db_term_list; /* Lock table */ - ets_insert_2_list_lock_tbl(table_id, p, bif_ix, ETS_INSERT_2_LIST_GLOBAL); - /* The operation must complete after this point */ + if (!ets_insert_2_list_lock_tbl(btid, p, bif_ix, ETS_INSERT_2_LIST_GLOBAL)) { + const Eterm fvalue = p->fvalue; + ASSERT(p->freason == (BADARG | EXF_HAS_EXT_INFO)); + + destroy_list = NULL; + ets_insert_2_list_destroy_copied_dbterms(meth, + compressed, + db_term_list); + /* Restore failure reason as we may have trapped during destroy */ + p->freason = BADARG | EXF_HAS_EXT_INFO; + p->fvalue = fvalue; + return THE_NON_VALUE; + } + if (is_insert_new) { if (ets_insert_new_2_dbterm_list_has_member(tb, db_term_list)) { + destroy_list = NULL; ets_insert_2_list_destroy_copied_dbterms(meth, compressed, db_term_list); cret = DB_ERROR_NONE_FALSE; - } else { + } + else { + destroy_list = NULL; ets_insert_2_list_insert_db_term_list(tb, db_term_list); } - } else { - ets_insert_2_list_insert_db_term_list(tb, db_term_list); } - if (tb->common.continuation != NULL) { - /* Uninstall the continuation from the table struct */ - tb->common.continuation = NULL; - if (is_insert_new) { - int* result_ptr = - ERTS_MAGIC_BIN_DATA(tb->common.continuation_res_bin); - *result_ptr = cret; - erts_bin_release(tb->common.continuation_res_bin); - } - tb->common.status |= tb->common.type & (DB_PRIVATE|DB_PROTECTED|DB_PUBLIC); - tb->common.status &= ~DB_BUSY; - erts_atomic_set_relb(&tb->common.continuation_state, (Sint)NULL); + else { + destroy_list = NULL; + ets_insert_2_list_insert_db_term_list(tb, db_term_list); } return ets_cret_to_return_value(NULL, cret); - /* The following code will be executed if the initiating process - is killed before an ets_insert_2_list_lock_tbl call has - succeeded */ YCF_SPECIAL_CODE_START(ON_DESTROY_STATE); { ets_insert_2_list_destroy_copied_dbterms(meth, compressed, - db_term_list); + destroy_list); } YCF_SPECIAL_CODE_END(); } #endif /* YCF_FUNCTIONS */ @@ -1887,48 +1928,76 @@ static void ets_insert_2_yield_free(void* data, void* ctx) static int ets_insert_2_list_yield_dtor(Binary* bin) { ets_insert_2_list_info* ctx = ERTS_MAGIC_BIN_DATA(bin); - if (ctx->status != ETS_INSERT_2_LIST_GLOBAL && - ctx->continuation_state != NULL) { - /* The operation has not been committed to the table and has - not completed*/ + if (ctx->continuation_state) { ets_insert_2_list_ycf_gen_destroy(ctx->continuation_state); } + erts_bin_release(ctx->btid); return 1; } -static void ets_insert_2_list_continuation(long *reds_ptr, - void** state, - void* extra_context) +#define ITERATIONS_PER_RED 8 + +static BIF_RETTYPE +ets_insert_2_list_continuation(Process* p, + ets_insert_2_list_info* ctx) { + long reds = ITERATIONS_PER_RED * ERTS_BIF_REDS_LEFT(p); + long init_reds; + BIF_RETTYPE ret; + + reds = DBG_RANDOM_REDS(reds, (Uint)ctx); + init_reds = reds; + + ERTS_LC_ASSERT(ctx->status != ETS_INSERT_2_LIST_GLOBAL + || DB_LOCK_FREE(tb) + || erts_lc_rwmtx_is_rwlocked(&ctx->tb->common.rwlock)); + ASSERT(ctx->continuation_state); + #if defined(DEBUG) && defined(ARCH_64) - ycf_debug_set_stack_start(reds_ptr); + ycf_debug_set_stack_start(&reds); #endif - ets_insert_2_list_ycf_gen_continue(reds_ptr, state, extra_context); + ret = ets_insert_2_list_ycf_gen_continue(&reds, + &ctx->continuation_state, + ctx); #if defined(DEBUG) && defined(ARCH_64) ycf_debug_reset_stack_start(); #endif -} -static int db_insert_new_2_res_bin_dtor(Binary *context_bin) -{ - (void)context_bin; - return 1; + if (ctx->continuation_state == NULL) { + if (is_value(ret)) { + ASSERT(ret == am_true || ret == am_false); + erts_atomic_set_nob(&ctx->return_value, ret); + } + if (ctx->status == ETS_INSERT_2_LIST_GLOBAL) { + DbTableCommon *tb = &ctx->tb->common; + if (tb->continuation_ctx) { + /* Uninstall the continuation from the table struct */ + ASSERT(!(tb->status & DB_DELETE)); + tb->status |= tb->type & (DB_PRIVATE|DB_PROTECTED|DB_PUBLIC); + tb->status &= ~DB_BUSY; + tb->continuation_ctx = NULL; + erts_bin_release(&(ERTS_MAGIC_BIN_FROM_DATA(ctx)->binary)); + } + } + } + BUMP_REDS(p, (init_reds - reds) / ITERATIONS_PER_RED); + return ret; } -#define ITERATIONS_PER_RED 8 - static BIF_RETTYPE ets_insert_2_list_driver(Process* p, Eterm tid, Eterm list, int is_insert_new) { - const long reds = ITERATIONS_PER_RED * ERTS_BIF_REDS_LEFT(p); - long nr_of_reductions = DBG_RANDOM_REDS(reds, (Uint)&p); - const long init_reds = nr_of_reductions; +#if defined(DEBUG) && defined(ARCH_64) + int dbg_ycf_stack_start; +#endif ets_insert_2_list_info* ctx = NULL; - ets_insert_2_list_info ictx; BIF_RETTYPE ret = THE_NON_VALUE; Eterm state_mref = list; Uint bix = (is_insert_new ? BIF_ets_insert_new_2 : BIF_ets_insert_2); + ets_insert_2_list_info ictx; + int do_trap; + if (is_internal_magic_ref(state_mref)) { Binary* state_bin = erts_magic_ref2bin(state_mref); if (ERTS_MAGIC_BIN_DESTRUCTOR(state_bin) != ets_insert_2_list_yield_dtor) { @@ -1938,59 +2007,60 @@ static BIF_RETTYPE ets_insert_2_list_driver(Process* p, erts_set_gc_state(p, 1); ctx = ERTS_MAGIC_BIN_DATA(state_bin); if (ctx->status == ETS_INSERT_2_LIST_GLOBAL) { - /* An operation that can be helped by other operations is - handled here */ - Uint freason; - int cret = DB_ERROR_NONE; - DbTable* tb; - /* First check if another process has completed the - operation without acquiring the lock */ - tb = db_get_table(p, tid, DB_READ_TBL_STRUCT, LCK_NOLOCK_ACCESS, - &freason); - ASSERT(tb || freason != TRAP); - if (tb != NULL && - (void*)erts_atomic_read_acqb(&tb->common.continuation_state) == - ctx->continuation_state) { - /* The lock has to be taken to complete the operation */ - if (NULL == (tb = db_get_table(p, tid, DB_WRITE, LCK_WRITE, &freason))) { - if (freason == TRAP){ - erts_set_gc_state(p, 0); - return db_bif_fail(p, freason, bix, NULL); - } - } - /* Must be done since the db_get_table call did not trap */ - if (tb != NULL) { + DbTable* tb = btid2tab(ctx->btid); + if (tb) { + db_lock(tb, LCK_WRITE); + if (ctx != tb->common.continuation_ctx) { db_unlock(tb, LCK_WRITE); + tb = NULL; } } - if (is_insert_new) { - int* res = ERTS_MAGIC_BIN_DATA(ctx->continuation_res_bin); - cret = *res; + if (!tb) { + /* + * Operation completed/aborted by someone else. + * Note: If insert was successful but table has been deleted, + * we still return success. It would be wrong to fail the insert + * if someone have seen the result (before the table was deleted). + */ + ret = erts_atomic_read_nob(&ctx->return_value); + if (is_value(ret)) { + ASSERT(ret == am_true || + (ret == am_false && is_insert_new)); + return ret; + } + else { + ASSERT(!tb || tb->common.status & DB_DELETE); + BIF_ERROR(p, BADARG | EXF_HAS_EXT_INFO); + } } - return ets_cret_to_return_value(NULL, cret); - } else { -#if defined(DEBUG) && defined(ARCH_64) - ycf_debug_set_stack_start(&nr_of_reductions); -#endif - ret = ets_insert_2_list_ycf_gen_continue(&nr_of_reductions, - &ctx->continuation_state, - ctx); -#if defined(DEBUG) && defined(ARCH_64) - ycf_debug_reset_stack_start(); -#endif + ASSERT((tb->common.status & (DB_WRITE|DB_BUSY)) + == DB_BUSY); } + ret = ets_insert_2_list_continuation(p, ctx); + ASSERT(ctx->continuation_state + || ret == am_true || ret == am_false || ret == THE_NON_VALUE); } else { /* Start call */ + long reds = ITERATIONS_PER_RED * ERTS_BIF_REDS_LEFT(p); + long init_reds; + + reds = DBG_RANDOM_REDS(reds, (Uint)p); + init_reds = reds; + ictx.continuation_state = NULL; ictx.status = ETS_INSERT_2_LIST_PROCESS_LOCAL; + erts_atomic_init_nob(&ictx.return_value, THE_NON_VALUE); ictx.tb = NULL; - ctx = &ictx; - DB_GET_TABLE(ctx->tb, tid, DB_READ_TBL_STRUCT, LCK_NOLOCK_ACCESS, bix, + ictx.tid = tid; + ictx.list = list; + DB_GET_TABLE(ictx.tb, tid, DB_READ_TBL_STRUCT, LCK_NOLOCK_ACCESS, bix, NULL, p); + ictx.btid = ictx.tb->common.btid; + ctx = &ictx; #if defined(DEBUG) && defined(ARCH_64) - ycf_debug_set_stack_start(&nr_of_reductions); + ycf_debug_set_stack_start(&dbg_ycf_stack_start); #endif - ret = ets_insert_2_list_ycf_gen_yielding(&nr_of_reductions, + ret = ets_insert_2_list_ycf_gen_yielding(&reds, &ctx->continuation_state, ctx, ets_insert_2_yield_alloc, @@ -2000,12 +2070,16 @@ static BIF_RETTYPE ets_insert_2_list_driver(Process* p, NULL, p, tid, + ctx->btid, ctx->tb, list, is_insert_new); #if defined(DEBUG) && defined(ARCH_64) ycf_debug_reset_stack_start(); #endif + ASSERT(ctx->continuation_state + || ret == am_true || ret == am_false || ret == THE_NON_VALUE); + if (ctx->continuation_state != NULL) { Binary* state_bin = erts_create_magic_binary(sizeof(ets_insert_2_list_info), ets_insert_2_list_yield_dtor); @@ -2013,40 +2087,37 @@ static BIF_RETTYPE ets_insert_2_list_driver(Process* p, state_mref = erts_mk_magic_ref(&hp, &MSO(p), state_bin); ctx = ERTS_MAGIC_BIN_DATA(state_bin); *ctx = ictx; + erts_refc_inc(&ctx->btid->intern.refc, 2); } - } - BUMP_REDS(p, (init_reds - nr_of_reductions) / ITERATIONS_PER_RED); - if (ctx->status == ETS_INSERT_2_LIST_GLOBAL && - ctx->continuation_state != NULL && - ctx->tb->common.continuation == NULL) { - /* Install the continuation in the table structure so other - threads can help */ - if (is_insert_new) { - Binary* bin = - erts_create_magic_binary(sizeof(int), - db_insert_new_2_res_bin_dtor); - Eterm* hp = HAlloc(p, ERTS_MAGIC_REF_THING_SIZE); - erts_mk_magic_ref(&hp, &MSO(p), bin); - erts_refc_inctest(&bin->intern.refc, 2); - ctx->tb->common.continuation_res_bin = bin; - ctx->continuation_res_bin = bin; + BUMP_REDS(p, (init_reds - reds) / ITERATIONS_PER_RED); + } + do_trap = (ctx->continuation_state != NULL); + if (do_trap) { + if (ctx->status == ETS_INSERT_2_LIST_GLOBAL && + !ctx->tb->common.continuation_ctx) { + /* Install the continuation in the table structure so other + threads can help */ + ctx->tb->common.status &= ~(DB_PRIVATE|DB_PROTECTED|DB_PUBLIC); + ctx->tb->common.status |= DB_BUSY; + ASSERT(ctx != &ictx); + erts_refc_inc(&(ERTS_MAGIC_BIN_FROM_DATA(ctx)->binary.intern.refc), 2); + ctx->tb->common.continuation_ctx = ctx; } - ctx->tb->common.continuation = ets_insert_2_list_continuation; - ctx->tb->common.status &= ~(DB_PRIVATE|DB_PROTECTED|DB_PUBLIC); - ctx->tb->common.status |= DB_BUSY; - erts_atomic_set_relb(&ctx->tb->common.continuation_state, - (Sint)ctx->continuation_state); } - if (ctx->status == ETS_INSERT_2_LIST_FAILED_TO_GET_LOCK_DESTROY) { - return ctx->destroy_return_value; + else if (is_non_value(ret)) { + ASSERT(p->freason != TRAP); + ERTS_BIF_ERROR_TRAPPED2(p, p->freason, BIF_TRAP_EXPORT(bix), + ctx->tid, ctx->list); } if (ctx->status == ETS_INSERT_2_LIST_GLOBAL) { db_unlock(ctx->tb, LCK_WRITE); } - if (ctx->continuation_state != NULL) { + if (do_trap) { erts_set_gc_state(p, 0); BIF_TRAP2(BIF_TRAP_EXPORT(bix), p, tid, state_mref); } + + ASSERT(ret == am_true || ret == am_false); return ret; } @@ -2433,8 +2504,7 @@ BIF_RETTYPE ets_new_2(BIF_ALIST_2) tb->common.status = status; tb->common.type = status; /* Note, 'type' is *read only* from now on... */ - tb->common.continuation = NULL; - erts_atomic_set_nob(&tb->common.continuation_state, (Sint)NULL); + tb->common.continuation_ctx = NULL; erts_refc_init(&tb->common.fix_count, 0); db_init_lock(tb, status & (DB_FINE_LOCKED|DB_FREQ_READ)); tb->common.keypos = keypos; @@ -2629,8 +2699,9 @@ BIF_RETTYPE ets_delete_1(BIF_ALIST_1) * Clear all access bits to prevent any ets operation to access the * table while it is being deleted. */ - tb->common.status &= ~(DB_PROTECTED|DB_PUBLIC|DB_PRIVATE); + tb->common.status &= ~(DB_PROTECTED | DB_PUBLIC | DB_PRIVATE | DB_BUSY); tb->common.status |= DB_DELETE; + cancel_trap_continuation(tb); if (tb->common.owner != BIF_P->common.id) { @@ -4660,8 +4731,10 @@ erts_db_process_exiting(Process *c_p, ErtsProcLocks c_p_locks, void **yield_stat break; } /* Clear all access bits. */ - tb->common.status &= ~(DB_PROTECTED | DB_PUBLIC | DB_PRIVATE); + tb->common.status &= ~(DB_PROTECTED | DB_PUBLIC | DB_PRIVATE + | DB_BUSY); tb->common.status |= DB_DELETE; + cancel_trap_continuation(tb); if (is_table_named(tb)) remove_named_tab(tb, 0); diff --git a/erts/emulator/beam/erl_db_util.h b/erts/emulator/beam/erl_db_util.h index 6ed1e15104..5289e93199 100644 --- a/erts/emulator/beam/erl_db_util.h +++ b/erts/emulator/beam/erl_db_util.h @@ -321,11 +321,7 @@ typedef struct db_table_common { int compress; /* For unfinished operations that needs to be helped */ - void (*continuation)(long *reds_ptr, - void** state, - void* extra_context); /* To help yielded process */ - erts_atomic_t continuation_state; - Binary* continuation_res_bin; + struct ets_insert_2_list_info* continuation_ctx; #ifdef ETS_DBG_FORCE_TRAP int dbg_force_trap; /* force trap on table lookup */ #endif diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl index dd8fe43da6..2b0dabc357 100644 --- a/lib/stdlib/test/ets_SUITE.erl +++ b/lib/stdlib/test/ets_SUITE.erl @@ -36,6 +36,7 @@ -export([dups/1, misc1/1, safe_fixtable/1, info/1, tab2list/1]). -export([info_binary_stress/1]). -export([info_whereis_busy/1]). +-export([insert_trap_delete/1, insert_trap_rename/1]). -export([tab2file/1, tab2file2/1, tabfile_ext1/1, tabfile_ext2/1, tabfile_ext3/1, tabfile_ext4/1, badfile/1]). -export([heavy_lookup/1, heavy_lookup_element/1, heavy_concurrent/1]). @@ -217,7 +218,9 @@ groups() -> [t_insert_list, t_insert_list_set, t_insert_list_bag, t_insert_list_duplicate_bag, t_insert_list_delete_set, t_insert_list_parallel, t_insert_list_delete_parallel, - t_insert_list_kill_process]}]. + t_insert_list_kill_process, + insert_trap_delete, + insert_trap_rename]}]. init_per_suite(Config) -> erts_debug:set_internal_state(available_internal_state, true), @@ -1907,7 +1910,7 @@ t_select_replace_next_bug(Config) when is_list(Config) -> %% OTP-17379 -t_select_pam_stack_overflow_bug(Config) -> +t_select_pam_stack_overflow_bug(_Config) -> T = ets:new(k, []), ets:insert(T,[{x,17}]), [{x,18}] = ets:select(T,[{{x,17}, [], [{{{element,1,'$_'},{const,18}}}]}]), @@ -5139,6 +5142,184 @@ info_whereis_busy(Config) when is_list(Config) -> ets:delete(T), ok. +%% Delete table during trapping ets:insert +insert_trap_delete(Config) when is_list(Config) -> + repeat_for_opts(fun(Opts) -> + [insert_trap_delete_run1({Opts,InsertFunc,Mode}) + || InsertFunc <- [insert,insert_new], + Mode <- [exit, delete]] + end, + [all_non_stim_types, write_concurrency, compressed]), + ok. + +insert_trap_delete_run1(Params) -> + NKeys = 50_000 + rand:uniform(50_000), + %% First measure how many traps the insert op will do + Traps0 = insert_trap_delete_run3(unlimited, Params, NKeys), + %% Then do again and delete table at different moments + Decr = (Traps0 div 5) + 1, + insert_trap_delete_run2(Traps0-1, Decr, Params, NKeys). + +insert_trap_delete_run2(Traps, _Decr, Params, NKeys) when Traps =< 1 -> + insert_trap_delete_run3(1, Params, NKeys), + ok; +insert_trap_delete_run2(Traps, Decr, Params, NKeys) -> + insert_trap_delete_run3(Traps, Params, NKeys), + insert_trap_delete_run2(Traps - Decr, Decr, Params, NKeys). + +insert_trap_delete_run3(Traps, {Opts, InsertFunc, Mode}, NKeys) -> + io:format("insert_trap_delete_run(~p, ~p, ~p) NKeys=~p\n", + [Traps, InsertFunc, Mode, NKeys]), + TabName = insert_trap_delete, + Tester = self(), + Tuples = [{K} || K <- lists:seq(1,NKeys)], + + OwnerFun = + fun() -> + erlang:trace(Tester, true, [running]), + ets_new(TabName, [named_table, public | Opts]), + Tester ! {ets_new, ets:whereis(TabName)}, + io:format("Wait for ets:~p/2 to yield...\n", [InsertFunc]), + GotTraps = repeat_while( + fun(N) -> + case receive_any() of + {trace, Tester, out, {ets,InsertFunc,2}} -> + case N of + Traps -> {false, Traps}; + _ -> {true, N+1} + end; + "Insert done" -> + io:format("Too late! Got ~p traps\n", [N]), + {false, N}; + _M -> + %%io:format("[~p] Ignored msg: ~p\n", [N,_M]), + {true, N} + end + end, + 0), + case Mode of + delete -> + io:format("Delete table and then exit...\n",[]), + ets:delete(TabName); + exit -> + io:format("Exit and let table die...\n",[]) + end, + Tester ! {traps, GotTraps} + end, + {Owner, Mon} = spawn_opt(OwnerFun, [link, monitor]), + + {ets_new, Tid} = receive_any(), + try ets:InsertFunc(TabName, Tuples) of + true -> + try ets:lookup(Tid, NKeys) of + [{NKeys}] -> ok + catch + error:badarg -> + %% Table must been deleted just after insert finished + undefined = ets:info(Tid, id), + undefined = ets:whereis(TabName) + end, + Owner ! "Insert done" + catch + error:badarg -> + %% Insert failed, table must have been deleted + undefined = ets:info(Tid, id), + undefined = ets:whereis(TabName) + end, + {traps, GotTraps} = receive_any(), + {'DOWN', Mon, process, Owner, _} = receive_any(), + undefined = ets:whereis(TabName), + undefined = ets:info(Tid, id), + GotTraps. + +%% Rename table during trapping ets:insert +insert_trap_rename(Config) when is_list(Config) -> + repeat_for_opts(fun(Opts) -> + [insert_trap_rename_run1(InsertFunc) + || InsertFunc <- [insert, insert_new]] + end, + [all_non_stim_types, write_concurrency, compressed]), + ok. + +insert_trap_rename_run1(InsertFunc) -> + NKeys = 50_000 + rand:uniform(50_000), + %% First measure how many traps the insert op will do + Traps0 = insert_trap_rename_run3(unlimited, InsertFunc, NKeys), + %% Then do again and rename table at different moments + Decr = (Traps0 div 5) + 1, + insert_trap_rename_run2(Traps0-1, Decr, InsertFunc, NKeys), + ok. + +insert_trap_rename_run2(Traps, _Decr, InsertFunc, NKeys) when Traps =< 1 -> + insert_trap_rename_run3(1, InsertFunc, NKeys), + ok; +insert_trap_rename_run2(Traps, Decr, InsertFunc, NKeys) -> + insert_trap_rename_run3(Traps, InsertFunc, NKeys), + insert_trap_rename_run2(Traps - Decr, Decr, InsertFunc, NKeys). + + +insert_trap_rename_run3(Traps, InsertFunc, NKeys) -> + io:format("insert_trap_rename_run(~p, ~p)\n", [Traps, InsertFunc]), + TabName = insert_trap_rename, + TabRenamed = insert_trap_rename_X, + Tester = self(), + Tuples = [{K} || K <- lists:seq(1,NKeys)], + + OwnerFun = + fun() -> + erlang:trace(Tester, true, [running]), + ets:new(TabName, [named_table, public]), + Tester ! {ets_new, ets:whereis(TabName)}, + io:format("Wait for ets:~p/2 to yield...\n", [InsertFunc]), + GotTraps = repeat_while( + fun(N) -> + case receive_any() of + {trace, Tester, out, {ets,InsertFunc,2}} -> + case N of + Traps -> {false, ok}; + _ -> {true, N+1} + end; + "Insert done" -> + io:format("Too late! Got ~p traps\n", [N]), + {false, N}; + _M -> + %%io:format("[~p] Ignored msg: ~p\n", [N,_M]), + {true, N} + end + end, + 0), + io:format("Rename table and wait...\n",[]), + ets:rename(TabName, TabRenamed), + ets:delete(TabRenamed, 42), + Tester ! {renamed, GotTraps}, + receive die -> ok end + end, + {Owner, Mon} = spawn_opt(OwnerFun, [link,monitor]), + + {ets_new, Tid} = receive_any(), + try ets:InsertFunc(TabName, Tuples) of + true -> + io:format("ets:~p succeeded\n", [InsertFunc]), + true = ets:member(Tid, 1), + true = ets:member(Tid, NKeys) + catch + error:badarg -> + io:format("ets:~p failed\n", [InsertFunc]), + false = ets:member(Tid, 1), + false = ets:member(Tid, NKeys) + end, + Owner ! "Insert done", + {renamed, GotTraps} = receive_any(), + [] = ets:lookup(Tid, 42), + undefined = ets:whereis(TabName), + Tid = ets:whereis(TabRenamed), + Owner ! die, + {'DOWN', Mon, process, Owner, _} = receive_any(), + undefined = ets:whereis(TabName), + undefined = ets:whereis(TabRenamed), + GotTraps. + + test_table_size_concurrency(Config) when is_list(Config) -> case erlang:system_info(schedulers) of 1 -> {skip,"Only valid on smp > 1 systems"}; @@ -8464,7 +8645,7 @@ repeat_for_permutations(Fun, List, N) -> receive_any() -> receive M -> - io:format("Process ~p got msg ~p\n", [self(),M]), + %%io:format("Process ~p got msg ~p\n", [self(),M]), M end. @@ -8702,6 +8883,8 @@ error_info(_Config) -> {insert, ['$Tab', [a|b]]}, {insert, ['$Tab', {a,b,c}], [no_fail]}, {insert, ['$Tab', [{a,b,c}]], [no_fail]}, + {insert, ['$Tab', [{a,b,c},{d,e,f}]], [no_fail]}, + {insert, ['$Tab', [{I,b,c} || I <- lists:seq(1,10_000)]], [no_fail]}, {insert_new, ['$Tab', bad_object]}, {insert_new, ['$Tab', {a,b,c}], [no_fail]}, @@ -8860,7 +9043,7 @@ error_info(_Config) -> [] -> ok; [_|_]=Errors -> - io:format("~p\n", [Errors]), + io:format("~P\n", [Errors, 100]), ct:fail({length(Errors),errors}) end. @@ -9085,7 +9268,7 @@ ets_apply(F, Args, Opts) -> end. ets_format_args(Args) -> - lists:join(", ", [io_lib:format("~p", [A]) || A <- Args]). + lists:join(", ", [io_lib:format("~P", [A,10]) || A <- Args]). %%% %%% Common utility functions. -- 2.35.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
