From 8bce2d5a342bda12aad6607312e3796a37d7c8eb Mon Sep 17 00:00:00 2001
From: Dan Gudmundsson <dgud@erlang.org>
Date: Tue, 6 Dec 2011 15:47:54 +0100
Subject: [mnesia] Fixed sticky read lock bug

wread on locks stuck at non-local node could return unexpected value.
Thanks to Magnus Henoch who posted a nice testcase showing the bug.
---
 lib/mnesia/src/mnesia_locker.erl             | 36 +++++++---------------------
 lib/mnesia/test/mnesia_atomicity_test.erl    |  8 ++++---
 lib/mnesia/test/mnesia_trans_access_test.erl |  9 +++++--
 3 files changed, 20 insertions(+), 33 deletions(-)

(limited to 'lib/mnesia')

diff --git a/lib/mnesia/src/mnesia_locker.erl b/lib/mnesia/src/mnesia_locker.erl
index 861f2df78d..5581c6c4a8 100644
--- a/lib/mnesia/src/mnesia_locker.erl
+++ b/lib/mnesia/src/mnesia_locker.erl
@@ -659,7 +659,7 @@ rwlock(Tid, Store, Oid) ->
 		yes ->
 		    {Ns, Majority} = w_nodes(Tab),
 		    check_majority(Majority, Tab, Ns),
-		    Res = get_rwlocks_on_nodes(Ns, rwlock, Node, Store, Tid, Oid),
+		    Res = get_rwlocks_on_nodes(Ns, make_ref(), Store, Tid, Oid),
 		    ?ets_insert(Store, {{locks, Tab, Key}, Lock}),
 		    Res;
 		no ->
@@ -889,37 +889,17 @@ get_wlocks_on_nodes([Node | Tail], Store, Request) ->
 get_wlocks_on_nodes([], _, _) ->
     ok.
 
-get_rwlocks_on_nodes([ReadNode|Tail], _Res, ReadNode, Store, Tid, Oid) ->
+get_rwlocks_on_nodes([ReadNode|Tail], Ref, Store, Tid, Oid) ->
     Op = {self(), {read_write, Tid, Oid}},
     {?MODULE, ReadNode} ! Op,
     ?ets_insert(Store, {nodes, ReadNode}),
-    Res = receive_wlocks([ReadNode], undefined, Store, Oid),
-    case node() of
-	ReadNode ->
-	    get_rwlocks_on_nodes(Tail, Res, ReadNode, Store, Tid, Oid);
-	_ ->
-	    get_wlocks_on_nodes(Tail, Store, {self(), {write, Tid, Oid}}),
-	    receive_wlocks(Tail, Res, Store, Oid)
-    end;
-get_rwlocks_on_nodes([Node | Tail], Res, ReadNode, Store, Tid, Oid) ->
-    Op = {self(), {write, Tid, Oid}},
-    {?MODULE, Node} ! Op,
-    ?ets_insert(Store, {nodes, Node}),
-    receive_wlocks([Node], undefined, Store, Oid),
-    if node() == Node ->
-	    get_rwlocks_on_nodes(Tail, Res, ReadNode, Store, Tid, Oid);
-       Res == rwlock -> %% Hmm
-	    Rest = lists:delete(ReadNode, Tail),
-	    Op2 = {self(), {read_write, Tid, Oid}},
-	    {?MODULE, ReadNode} ! Op2,
-	    ?ets_insert(Store, {nodes, ReadNode}),
-	    get_wlocks_on_nodes(Rest, Store, {self(), {write, Tid, Oid}}),
-	    receive_wlocks([ReadNode|Rest], undefined, Store, Oid);
-       true ->
-	    get_wlocks_on_nodes(Tail, Store, {self(), {write, Tid, Oid}}),
-	    receive_wlocks(Tail, Res, Store, Oid)
+    case receive_wlocks([ReadNode], Ref, Store, Oid) of
+	Ref ->
+	    get_rwlocks_on_nodes(Tail, Ref, Store, Tid, Oid);
+	Res ->
+	    get_wlocks_on_nodes(Tail, Res, Store, {self(), {write, Tid, Oid}}, Oid)
     end;
-get_rwlocks_on_nodes([],Res,_,_,_,_) ->
+get_rwlocks_on_nodes([],Res,_,_,_) ->
     Res.
 
 receive_wlocks([], Res, _Store, _Oid) ->
diff --git a/lib/mnesia/test/mnesia_atomicity_test.erl b/lib/mnesia/test/mnesia_atomicity_test.erl
index 39fb7d4642..06c4d16d71 100644
--- a/lib/mnesia/test/mnesia_atomicity_test.erl
+++ b/lib/mnesia/test/mnesia_atomicity_test.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 1997-2010. All Rights Reserved.
+%% Copyright Ericsson AB 1997-2011. 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
@@ -535,8 +535,10 @@ start_lock_waiter(BlockOpA, BlockOpB, Config) ->
 	    BlockOpA == rt, BlockOpB /= sw -> 1;
 	    true -> 2
 	end,
-    ?match_multi_receive([{'EXIT', A, {atomic, ExpectedCounter}},
-			  {'EXIT', B, killed}]),
+    receive {'EXIT', B, _} -> ok
+    after 3000 -> ?error("Timeout~n", []) end,
+    receive {'EXIT', A, Exp1} -> ?match({atomic, ExpectedCounter}, Exp1)
+    after 3000 -> ?error("Timeout~n", []) end,
 
     %% the expected result depends on the transaction of
     %% fun A - when that doesn't change the object in the
diff --git a/lib/mnesia/test/mnesia_trans_access_test.erl b/lib/mnesia/test/mnesia_trans_access_test.erl
index f826d7fb34..c040d0ca3f 100644
--- a/lib/mnesia/test/mnesia_trans_access_test.erl
+++ b/lib/mnesia/test/mnesia_trans_access_test.erl
@@ -132,9 +132,9 @@ read(Config) when is_list(Config) ->
 
 wread(suite) -> [];
 wread(Config) when is_list(Config) ->
-    [Node1] = Nodes = ?acquire_nodes(1, Config),
+    [_N1,N2] = Nodes = ?acquire_nodes(2, Config),
     Tab = wread,
-    Schema = [{name, Tab}, {type, set}, {attributes, [k, v]}, {ram_copies, [Node1]}],
+    Schema = [{name, Tab}, {type, set}, {attributes, [k, v]}, {ram_copies, Nodes}],
     ?match({atomic, ok},  mnesia:create_table(Schema)),
 
     OneRec = {Tab, 1, 2},
@@ -159,6 +159,11 @@ wread(Config) when is_list(Config) ->
 	   mnesia:transaction(fun() -> mnesia:wread({Tab, 1}) end)),
 
     ?match({'EXIT', {aborted, no_transaction}},  mnesia:wread({Tab, 1})),
+
+    ?match({atomic, ok},
+	   mnesia:transaction(fun() -> mnesia:write(Tab, {Tab, 42, a}, sticky_write) end)),
+    ?match({atomic, [{Tab,42, a}]},
+	   rpc:call(N2, mnesia, transaction, [fun() -> mnesia:wread({Tab, 42}) end])),
     ?verify_mnesia(Nodes, []).
 
 %% Delete record
-- 
cgit v1.2.3