aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorErlang/OTP <[email protected]>2016-07-14 11:22:31 +0200
committerErlang/OTP <[email protected]>2016-07-14 11:22:31 +0200
commitc7d729da231c40a656725f2e9bb84c40a6bddc7d (patch)
treeda2003f9e74c8c9fb7bc8f595144cf7805e0bb76
parent23bb0ba4db2351c2b89e8fb80371fad74d58d544 (diff)
parent30cb490ed08d358d584a90aac4819c85ddf6fcce (diff)
downloadotp-c7d729da231c40a656725f2e9bb84c40a6bddc7d.tar.gz
otp-c7d729da231c40a656725f2e9bb84c40a6bddc7d.tar.bz2
otp-c7d729da231c40a656725f2e9bb84c40a6bddc7d.zip
Merge branch 'sverker/update_counter-deadlock/ERL-188/OTP-13731' into maint-19
* sverker/update_counter-deadlock/ERL-188/OTP-13731: erts: Add test ets_SUITE:update_counter_table_growth erts: Fix deadlock in ets:update_counter/4 erts: Optimize db_finalize_dbterm_hash
-rw-r--r--erts/emulator/beam/erl_db_hash.c42
-rw-r--r--erts/emulator/beam/erl_db_util.h1
-rw-r--r--lib/stdlib/test/ets_SUITE.erl13
3 files changed, 38 insertions, 18 deletions
diff --git a/erts/emulator/beam/erl_db_hash.c b/erts/emulator/beam/erl_db_hash.c
index 74979f984a..12ae086b31 100644
--- a/erts/emulator/beam/erl_db_hash.c
+++ b/erts/emulator/beam/erl_db_hash.c
@@ -2866,15 +2866,7 @@ db_lookup_dbterm_hash(Process *p, DbTable *tbl, Eterm key, Eterm obj,
q->hvalue = hval;
q->next = NULL;
*bp = b = q;
-
- {
- int nitems = erts_smp_atomic_inc_read_nob(&tb->common.nitems);
- int nactive = NACTIVE(tb);
-
- if (nitems > nactive * (CHAIN_LEN + 1) && !IS_FIXED(tb)) {
- grow(tb, nactive);
- }
- }
+ flags |= DB_INC_TRY_GROW;
} else {
HashDbTerm *q, *next = b->next;
@@ -2910,6 +2902,7 @@ db_finalize_dbterm_hash(int cret, DbUpdateHandle* handle)
HashDbTerm **bp = (HashDbTerm **) handle->bp;
HashDbTerm *b = *bp;
erts_smp_rwmtx_t* lck = (erts_smp_rwmtx_t*) handle->lck;
+ HashDbTerm* free_me = NULL;
ERTS_SMP_LC_ASSERT(IS_HASH_WLOCKED(tb, lck)); /* locked by db_lookup_dbterm_hash */
@@ -2921,21 +2914,34 @@ db_finalize_dbterm_hash(int cret, DbUpdateHandle* handle)
b->hvalue = INVALID_HASH;
} else {
*bp = b->next;
- free_term(tb, b);
+ free_me = b;
}
WUNLOCK_HASH(lck);
erts_smp_atomic_dec_nob(&tb->common.nitems);
try_shrink(tb);
- } else if (handle->flags & DB_MUST_RESIZE) {
- db_finalize_resize(handle, offsetof(HashDbTerm,dbterm));
- WUNLOCK_HASH(lck);
-
- free_term(tb, b);
- }
- else {
- WUNLOCK_HASH(lck);
+ } else {
+ if (handle->flags & DB_MUST_RESIZE) {
+ db_finalize_resize(handle, offsetof(HashDbTerm,dbterm));
+ free_me = b;
+ }
+ if (handle->flags & DB_INC_TRY_GROW) {
+ int nactive;
+ int nitems = erts_smp_atomic_inc_read_nob(&tb->common.nitems);
+ WUNLOCK_HASH(lck);
+ nactive = NACTIVE(tb);
+
+ if (nitems > nactive * (CHAIN_LEN + 1) && !IS_FIXED(tb)) {
+ grow(tb, nactive);
+ }
+ } else {
+ WUNLOCK_HASH(lck);
+ }
}
+
+ if (free_me)
+ free_term(tb, free_me);
+
#ifdef DEBUG
handle->dbterm = 0;
#endif
diff --git a/erts/emulator/beam/erl_db_util.h b/erts/emulator/beam/erl_db_util.h
index 60f7067d70..4acedbfed0 100644
--- a/erts/emulator/beam/erl_db_util.h
+++ b/erts/emulator/beam/erl_db_util.h
@@ -79,6 +79,7 @@ typedef union db_table DbTable;
#define DB_MUST_RESIZE 1
#define DB_NEW_OBJECT 2
+#define DB_INC_TRY_GROW 4
/* Info about a database entry while it's being updated
* (by update_counter or update_element)
diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl
index 8c1c625676..b02d17bdb6 100644
--- a/lib/stdlib/test/ets_SUITE.erl
+++ b/lib/stdlib/test/ets_SUITE.erl
@@ -47,6 +47,7 @@
fixtable_next/1, fixtable_insert/1, rename/1, rename_unnamed/1, evil_rename/1,
update_element/1, update_counter/1, evil_update_counter/1, partly_bound/1, match_heavy/1]).
-export([update_counter_with_default/1]).
+-export([update_counter_table_growth/1]).
-export([member/1]).
-export([memory/1]).
-export([select_fail/1]).
@@ -100,6 +101,7 @@
heavy_lookup_element_do/1, member_do/1, otp_5340_do/1, otp_7665_do/1, meta_wb_do/1,
do_heavy_concurrent/1, tab2file2_do/2, exit_large_table_owner_do/2,
types_do/1, sleeper/0, memory_do/1, update_counter_with_default_do/1,
+ update_counter_table_growth_do/1,
ms_tracee_dummy/1, ms_tracee_dummy/2, ms_tracee_dummy/3, ms_tracee_dummy/4
]).
@@ -137,6 +139,7 @@ all() ->
rename, rename_unnamed, evil_rename, update_element,
update_counter, evil_update_counter,
update_counter_with_default, partly_bound,
+ update_counter_table_growth,
match_heavy, {group, fold}, member, t_delete_object,
t_init_table, t_whitebox, t_delete_all_objects,
t_insert_list, t_test_ms, t_select_delete, t_ets_dets,
@@ -1955,6 +1958,16 @@ update_counter_with_default_do(Opts) ->
ok.
+update_counter_table_growth(_Config) ->
+ repeat_for_opts(update_counter_table_growth_do).
+
+update_counter_table_growth_do(Opts) ->
+ Set = ets_new(b, [set | Opts]),
+ [ets:update_counter(Set, N, {2, 1}, {N, 1}) || N <- lists:seq(1,10000)],
+ OrderedSet = ets_new(b, [ordered_set | Opts]),
+ [ets:update_counter(OrderedSet, N, {2, 1}, {N, 1}) || N <- lists:seq(1,10000)],
+ ok.
+
%% Check that a first-next sequence always works on a fixed table.
fixtable_next(Config) when is_list(Config) ->
repeat_for_opts(fixtable_next_do, [write_concurrency,all_types]).