Bug 1720232 SQLite calls could timeout in starvation situations.

Some of our servers could cause random failures when trying to generate many key pairs from multiple threads. This is caused because some threads would starve long enough for them to give up on getting a begin transaction on sqlite. sqlite only allows one transaction at a time.

Also, there were some bugs in error handling of the broken transaction case where NSS would try to cancel a transation after the begin failed (most cases were correct, but one case in particular was problematic).

Differential Revision: https://phabricator.services.mozilla.com/D120032

1 files changed, 42 insertions, 17 deletions

diff --git a/tests/dbtests/dbtests.sh b/tests/dbtests/dbtests.sh
index 9dcf738c4..61a3888c6 100755
--- a/tests/dbtests/dbtests.sh
+++ b/tests/dbtests/dbtests.sh
@@ -8,7 +8,7 @@
 #
 # mozilla/security/nss/tests/dbtest/dbtest.sh
 #
-# Certificate generating and handeling for NSS QA, can be included 
+# Certificate generating and handeling for NSS QA, can be included
 # multiple times from all.sh and the individual scripts
 #
 # needs to work on all Unix and Windows platforms
@@ -50,6 +50,7 @@ dbtest_init()
   RONLY_DIR=${HOSTDIR}/ronlydir
   EMPTY_DIR=${HOSTDIR}/emptydir
   CONFLICT_DIR=${HOSTDIR}/conflictdir
+  THREAD_DIR=${HOSTDIR}/threadir
 
   html_head "CERT and Key DB Tests"
 
@@ -61,7 +62,7 @@ dbtest_init()
 ########################################################################
 dbtest_cleanup()
 {
-  html "</TABLE><BR>" 
+  html "</TABLE><BR>"
   cd ${QADIR}
   chmod a+rw $RONLY_DIR
   . common/cleanup.sh
@@ -78,21 +79,21 @@ dbtest_main()
 {
     cd ${HOSTDIR}
 
-    
+
     Echo "test opening the database read/write in a nonexisting directory"
     ${BINDIR}/certutil -L -X -d ./non_existent_dir
     ret=$?
     if [ $ret -ne 255 ]; then
       html_failed "Certutil succeeded in a nonexisting directory $ret"
     else
-      html_passed "Certutil didn't work in a nonexisting dir $ret" 
+      html_passed "Certutil didn't work in a nonexisting dir $ret"
     fi
     ${BINDIR}/dbtest -r -d ./non_existent_dir
     ret=$?
     if [ $ret -ne 46 ]; then
       html_failed "Dbtest readonly succeeded in a nonexisting directory $ret"
     else
-      html_passed "Dbtest readonly didn't work in a nonexisting dir $ret" 
+      html_passed "Dbtest readonly didn't work in a nonexisting dir $ret"
     fi
 
     Echo "test force opening the database in a nonexisting directory"
@@ -106,7 +107,7 @@ dbtest_main()
 
     Echo "test opening the database readonly in an empty directory"
     mkdir $EMPTY_DIR
-    ${BINDIR}/tstclnt -h  ${HOST}  -d $EMPTY_DIR 
+    ${BINDIR}/tstclnt -h  ${HOST}  -d $EMPTY_DIR
     ret=$?
     if [ $ret -ne 1 ]; then
       html_failed "Tstclnt succeded in an empty directory $ret"
@@ -118,7 +119,7 @@ dbtest_main()
     if [ $ret -ne 46 ]; then
       html_failed "Dbtest readonly succeeded in an empty directory $ret"
     else
-      html_passed "Dbtest readonly didn't work in an empty dir $ret" 
+      html_passed "Dbtest readonly didn't work in an empty dir $ret"
     fi
     rm -rf $EMPTY_DIR/* 2>/dev/null
     ${BINDIR}/dbtest -i -d $EMPTY_DIR
@@ -126,7 +127,7 @@ dbtest_main()
     if [ $ret -ne 0 ]; then
       html_failed "Dbtest logout after empty DB Init loses key $ret"
     else
-      html_passed "Dbtest logout after empty DB Init has key" 
+      html_passed "Dbtest logout after empty DB Init has key"
     fi
     rm -rf $EMPTY_DIR/* 2>/dev/null
     ${BINDIR}/dbtest -i -p pass -d $EMPTY_DIR
@@ -134,12 +135,12 @@ dbtest_main()
     if [ $ret -ne 0 ]; then
       html_failed "Dbtest password DB Init loses needlogin state $ret"
     else
-      html_passed "Dbtest password DB Init maintains needlogin state" 
+      html_passed "Dbtest password DB Init maintains needlogin state"
     fi
     rm -rf $EMPTY_DIR/* 2>/dev/null
     ${BINDIR}/certutil -D -n xxxx -d $EMPTY_DIR #created DB
     ret=$?
-    if [ $ret -ne 255 ]; then 
+    if [ $ret -ne 255 ]; then
         html_failed "Certutil succeeded in deleting a cert in an empty directory $ret"
     else
         html_passed "Certutil didn't work in an empty dir $ret"
@@ -176,7 +177,7 @@ dbtest_main()
     if [ $ret -ne 46 ]; then
       html_failed "Dbtest r/w succeeded in a readonly directory $ret"
     else
-      html_passed "Dbtest r/w didn't work in an readonly dir $ret" 
+      html_passed "Dbtest r/w didn't work in an readonly dir $ret"
     fi
     else
       html_passed "Skipping Dbtest r/w in a readonly dir because user is root"
@@ -190,9 +191,9 @@ dbtest_main()
       html_passed "Certutil didn't work in an readonly dir $ret"
     fi
     else
-        html_passed "Skipping Certutil delete cert in a readonly directory test because user is root" 
+        html_passed "Skipping Certutil delete cert in a readonly directory test because user is root"
     fi
-    
+
     Echo "test opening the database ronly in a readonly directory"
 
     ${BINDIR}/dbtest -d $RONLY_DIR -r
@@ -200,7 +201,7 @@ dbtest_main()
     if [ $ret -ne 0 ]; then
       html_failed "Dbtest readonly failed in a readonly directory $ret"
     else
-      html_passed "Dbtest readonly succeeded in a readonly dir $ret" 
+      html_passed "Dbtest readonly succeeded in a readonly dir $ret"
     fi
 
     Echo "test force opening the database  r/w in a readonly directory"
@@ -223,7 +224,7 @@ dbtest_main()
     ret=$?
     if [ $ret -ne 0 ]; then
       html_failed "Nicknane conflict test failed, couldn't create database $ret"
-    else 
+    else
       ${BINDIR}/certutil -A -n alice -t ,, -i ${R_ALICEDIR}/Alice.cert -d ${CONFLICT_DIR}
       ret=$?
       if [ $ret -ne 0 ]; then
@@ -252,7 +253,7 @@ dbtest_main()
     else
       html_passed "Nicknane conflict test-setting nickname conflict was correctly rejected"
     fi
-    # import a token private key and make sure the corresponding public key is 
+    # import a token private key and make sure the corresponding public key is
     # created
     ${BINDIR}/pk11importtest -d ${CONFLICT_DIR} -f ${R_PWFILE}
     ret=$?
@@ -261,10 +262,34 @@ dbtest_main()
     else
       html_passed "Importing Token Private Key correctly creates the corrresponding Public Key"
     fi
+
+
+    if [ "${NSS_DEFAULT_DB_TYPE}" = "sql" ] ; then
+      LOOPS=${NSS_SDB_THREAD_LOOPS-7}
+      THREADS=${NSS_SDB_THREAD_THREADS-30}
+      mkdir -p ${THREAD_DIR}
+      Echo "testing for thread starvation while creating keys"
+      ${BINDIR}/certutil -N -d ${THREAD_DIR} --empty-password
+      ${BINDIR}/sdbthreadtst -l ${LOOPS} -t ${THREADS} -d ${THREAD_DIR}
+      ret=$?
+      case "$ret" in
+      "0")
+         html_passed "Successfully completed ${LOOPS} loops in ${THREADS} threads without failure."
+         ;;
+      "2")
+         html_failed "sdbthreadtst failed for some environment reason (like lack of memory)"
+         ;;
+      "1")
+         html_failed "sdbthreadtst failed do to starvation using ${LOOPS} loops and ${THREADS} threads."
+         ;;
+      *)
+         html_failed "sdbthreadtst failed with an unrecognized error code."
+      esac
+    fi
 }
 
 ################## main #################################################
 
-dbtest_init 
+dbtest_init
 dbtest_main 2>&1
 dbtest_cleanup