From 7c9da71b18dd2921672e4ba3e15c11cda1ade4da Mon Sep 17 00:00:00 2001
From: Jon Meredith <jmeredith@thehive.com>
Date: Fri, 19 Jun 2009 08:44:43 -0600
Subject: [PATCH] The read lock checks for the G_DATABASES rwlock were
 incorrect.  Multiple readers are possible.  Now the check for exclusive
 port/thread is only done on writes.

Added an integration test that reproduces conditions seen during the local-storage-ops-
in-coord branch.
---
 c_src/bdberl_drv.c        | 18 +++++++-----
 int_test/bug200_SUITE.erl | 62 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 8 deletions(-)
 create mode 100644 int_test/bug200_SUITE.erl

diff --git a/c_src/bdberl_drv.c b/c_src/bdberl_drv.c
index 8bfbf52..1a66bea 100644
--- a/c_src/bdberl_drv.c
+++ b/c_src/bdberl_drv.c
@@ -139,8 +139,8 @@ static Database*     G_DATABASES        = 0;
 static Database*     G_DATABASES_SHADOW = 0;
 static unsigned int  G_DATABASES_SIZE   = 0;
 static ErlDrvRWLock* G_DATABASES_RWLOCK = 0;
-static volatile ErlDrvTid     G_DATABASES_RWLOCK_TID = 0;
-static volatile ErlDrvPort    G_DATABASES_RWLOCK_PORT = 0;
+static volatile ErlDrvTid     G_DATABASES_RWLOCK_TID = 0;  /* TID for writers */
+static volatile ErlDrvPort    G_DATABASES_RWLOCK_PORT = 0; /* Port for writers */
 static hive_hash*    G_DATABASES_NAMES  = 0;
 
 
@@ -220,26 +220,26 @@ static void DBGCMDRC(PortData *d, int rc);
 
 void READ_LOCK_DATABASES(void *x, ErlDrvPort P)
 {
+#ifdef DEBUG
     ErlDrvTid self = erl_drv_thread_self();                             
+#endif
     DBG("threadid %p port %p: read locking G_DATABASES\r\n", self, P);       
     erl_drv_rwlock_rlock(G_DATABASES_RWLOCK);                                        
     assert(0 == G_DATABASES_RWLOCK_TID);                                           
     assert(0 == G_DATABASES_RWLOCK_PORT);                                          
-    G_DATABASES_RWLOCK_TID = self;                                
-    G_DATABASES_RWLOCK_PORT = P;                                                   
     assert(0 == memcmp(G_DATABASES, G_DATABASES_SHADOW, sizeof(G_DATABASES[0])*G_DATABASES_SIZE)); 
     DBG("threadid %p port %p: read locked G_DATABASES\r\n", self, P);   
 }
 
 void READ_UNLOCK_DATABASES(void *x, ErlDrvPort P)                                                
 {
+#ifdef DEBUG
     ErlDrvTid self = erl_drv_thread_self();                         
+#endif
     DBG("threadid %p port %p: read unlocking G_DATABASES\r\n", self, P);      
-    assert(erl_drv_thread_self() == G_DATABASES_RWLOCK_TID);                       
-    assert(P == G_DATABASES_RWLOCK_PORT);                                          
+    assert(0 == G_DATABASES_RWLOCK_TID);                                           
+    assert(0 == G_DATABASES_RWLOCK_PORT);                                          
     assert(0 == memcmp(G_DATABASES, G_DATABASES_SHADOW, sizeof(G_DATABASES[0])*G_DATABASES_SIZE)); 
-    G_DATABASES_RWLOCK_TID = 0;                                                    
-    G_DATABASES_RWLOCK_PORT = 0;                                                   
     erl_drv_rwlock_runlock(G_DATABASES_RWLOCK);                                      
     DBG("threadid %p port %p: read unlocked G_DATABASES\r\n", self, P);
 }
@@ -263,7 +263,9 @@ void WRITE_LOCK_DATABASES(void *x, ErlDrvPort P)
       
 void WRITE_UNLOCK_DATABASES(void *x, ErlDrvPort P)                                                
 {
+#ifdef DEBUG
     ErlDrvTid self = erl_drv_thread_self();                         
+#endif
     DBG("threadid %p port %p: write unlocking G_DATABASES\r\n", self, P);
     assert(erl_drv_thread_self() == G_DATABASES_RWLOCK_TID);                       
     assert(P == G_DATABASES_RWLOCK_PORT);                                          
diff --git a/int_test/bug200_SUITE.erl b/int_test/bug200_SUITE.erl
new file mode 100644
index 0000000..7e929fb
--- /dev/null
+++ b/int_test/bug200_SUITE.erl
@@ -0,0 +1,62 @@
+%% -------------------------------------------------------------------
+%%
+%% bdberl: Port Driver Bug200 tests
+%% Copyright (c) 2008 The Hive.  All rights reserved.
+%%
+%% -------------------------------------------------------------------
+-module(bug200_SUITE).
+
+-compile(export_all).
+
+-define(PROCS, 32).
+all() ->
+    [bug200].
+
+bug200(_Config) ->
+    %% Spin up 15 processes (async thread pool is 10)
+    start_procs(?PROCS),
+    wait_for_finish(?PROCS).
+
+start_procs(0) ->
+    ok;
+start_procs(Count) ->
+    spawn_link(?MODULE, bug200_run, [self()]),
+    start_procs(Count-1).
+
+wait_for_finish(0) ->
+    ok;
+wait_for_finish(Count) ->
+    receive
+        {finished, Pid} ->
+            ct:print("~p is done; ~p remaining.\n", [Pid, Count-1]),
+            wait_for_finish(Count-1)
+    end.
+
+bug200_run(Owner) ->
+    %% Seed the RNG
+    {A1, A2, A3} = now(),
+    random:seed(A1, A2, A3),
+
+    %% Start bug200ing
+    bug200_incr_loop(Owner, 1000).
+
+bug200_incr_loop(Owner, 0) ->
+    Owner ! {finished, self()};
+bug200_incr_loop(Owner, Count) ->
+    % ct:print("~p", [Count]),
+    %% Choose random key
+    Key = random:uniform(1200),
+    File = "bug200_" ++ integer_to_list(random:uniform(16)) ++ ".bdb",
+    %% Start a txn that will read the current value of the key and increment by 1
+    F = fun(_Key, Value) ->
+            case Value of
+                not_found -> 0;
+                Value     -> Value + 1
+            end
+        end,
+    %% Open up a port and database
+    {ok, DbRef} = bdberl:open(File, btree),
+    {ok, _} = bdberl:update(DbRef, Key, F),
+    ok = bdberl:close(DbRef),
+    bug200_incr_loop(Owner, Count-1).
+