Add tests and fix bugs in the new cross-thread lock resolution code.

When an unlock fails, do not leak file descriptors (ticket #1611).
But we really ought to report SQLITE_MISUSE or some other error instead
of just returning SQLITE_OK. (CVS 2945)

FossilOrigin-Name: f68e05cb2be65fad43fac823b2a9c53b6d2e797d
This commit is contained in:
drh 2006-01-15 02:30:57 +00:00
parent 029b44bdd3
commit 64b1bea3ba
6 changed files with 355 additions and 41 deletions

View File

@ -1,5 +1,5 @@
C Documentation\supdates.\s\sFix\sto\sdate.c.\s\sBut\smost\simportantly:\sdatabase\nconnections\sare\snow\sallowed\sto\schange\sthreads\sas\slong\sas\sthey\sare\snot\nholding\sa\slock.\s(CVS\s2944)
D 2006-01-15T00:13:16
C Add\stests\sand\sfix\sbugs\sin\sthe\snew\scross-thread\slock\sresolution\scode.\nWhen\san\sunlock\sfails,\sdo\snot\sleak\sfile\sdescriptors\s(ticket\s#1611).\nBut\swe\sreally\sought\sto\sreport\sSQLITE_MISUSE\sor\ssome\sother\serror\sinstead\nof\sjust\sreturning\sSQLITE_OK.\s(CVS\s2945)
D 2006-01-15T02:30:58
F Makefile.in ab3ffd8d469cef4477257169b82810030a6bb967
F Makefile.linux-gcc aee18d8a05546dcf1888bd4547e442008a49a092
F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028
@ -55,7 +55,7 @@ F src/os.h 9debc3d3ca4cdafde222a0ea74a4c8415aef4f22
F src/os_common.h 6b76efa9b252e288de53b202ed5a0d48f48dc8db
F src/os_test.c 49833426101f99aee4bb5f6a44b7c4b2029fda1c
F src/os_test.h 903c93554c23d88f34f667f1979e4a1cee792af3
F src/os_unix.c e6ff5035b3d06fc69773e52e1bc6f7f7ccace084
F src/os_unix.c a1245b110150254ed2186d43edad9e00f21bc869
F src/os_unix.h 5768d56d28240d3fe4537fac08cc85e4fb52279e
F src/os_win.c cd4ca2753aeaad11f5c9b9b6ef28752f45ed4529
F src/os_win.h 41a946bea10f61c158ce8645e7646b29d44f122b
@ -73,10 +73,10 @@ F src/sqlite.h.in 492580f7e3ff71eb43193eb7bb98e2d549889ce3
F src/sqliteInt.h ed482d6de58fa79000f9c3bb00d7740126fb55fb
F src/table.c 486dcfce532685b53b5a2b5da8bba0ded6fb2316
F src/tclsqlite.c d650bea0248fc0a310ddc2cb94273a3a5021fddf
F src/test1.c 22709543ac2f7a5979ec51c3130ba93cf6e82951
F src/test1.c d21e94ea95e76d8e838792806937332c5693dbf0
F src/test2.c ca74a1d8aeb7d9606e8f6b762c5daf85c1a3f92b
F src/test3.c 9742aa146eb750cab81c1d5605286c3a0eb88054
F src/test4.c 0f95de81629a53fc6ab104f64a7ccb1dcbb8af90
F src/test4.c 6633cb7b4a7429e938804a34f688d118c8b7f8e1
F src/test5.c 7162f8526affb771c4ed256826eee7bb9eca265f
F src/test6.c 74d91b487c68154156eded457925d96aa2a3fdbb
F src/test7.c c87fd373a613986113717ba51e6a5ad86407c1f7
@ -236,6 +236,7 @@ F test/tclsqlite.test 2c4b5fb2f21e6740479463c263f3020f08e472d7
F test/temptable.test 7927261befdbc7b0a7ffebb85ecc70a74fa7b15b
F test/tester.tcl ed59b4eaa180728e4be1c55dc2823d66e03ae79a
F test/thread1.test 776c9e459b75ba905193b351926ac4019b049f35
F test/thread2.test c88da55fb60d5975be91f1e2942a5e267c33f8ed
F test/threadtest1.c 6029d9c5567db28e6dc908a0c63099c3ba6c383b
F test/threadtest2.c 97a830d53c24c42290501fdfba4a6e5bdd34748b
F test/tkt1435.test f768e5415d102fa1d8de3f548469d8fd1b79abd8
@ -340,7 +341,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9
F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
F www/version3.tcl a99cf5f6d8bd4d5537584a2b342f0fb9fa601d8b
F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513
P 8e79a0c24a03ccf960d6ccfb7c6b9b0f7c614e9b
R 9d07ce30bc69528746c99ed3d4ba8921
P 03c422ecb508dd84dfafc8b7a0b790a43f5dadda
R 1642e8fb6600fd54b6368241cbaa46c5
U drh
Z 392783876068e8090d13d0e230d0bba8
Z 4fe4a4d33caa52376ec7d83da72f0c2f

View File

@ -1 +1 @@
03c422ecb508dd84dfafc8b7a0b790a43f5dadda
f68e05cb2be65fad43fac823b2a9c53b6d2e797d

View File

@ -486,7 +486,7 @@ static void releaseOpenCnt(struct openCnt *pOpen){
pOpen->nRef--;
if( pOpen->nRef==0 ){
sqlite3HashInsert(&openHash, &pOpen->key, sizeof(pOpen->key), 0);
sqliteFree(pOpen->aPending);
free(pOpen->aPending);
sqliteFree(pOpen);
}
}
@ -581,6 +581,24 @@ exit_findlockinfo:
return rc;
}
#ifdef SQLITE_DEBUG
/*
** Helper function for printing out trace information from debugging
** binaries. This returns the string represetation of the supplied
** integer lock-type.
*/
static const char *locktypeName(int locktype){
switch( locktype ){
case NO_LOCK: return "NONE";
case SHARED_LOCK: return "SHARED";
case RESERVED_LOCK: return "RESERVED";
case PENDING_LOCK: return "PENDING";
case EXCLUSIVE_LOCK: return "EXCLUSIVE";
}
return "ERROR";
}
#endif
/*
** If we are currently in a different thread than the thread that the
** unixFile argument belongs to, then transfer ownership of the unixFile
@ -596,6 +614,7 @@ exit_findlockinfo:
*/
#ifdef SQLITE_UNIX_THREADS
static int transferOwnership(unixFile *pFile){
int rc;
pthread_t hSelf;
if( threadsOverrideEachOthersLocks ){
/* Ownership transfers not needed on this system */
@ -604,15 +623,21 @@ static int transferOwnership(unixFile *pFile){
hSelf = pthread_self();
if( pthread_equal(pFile->tid, hSelf) ){
/* We are still in the same thread */
TRACE1("No-transfer, same thread\n");
return SQLITE_OK;
}
if( pFile->locktype!=NO_LOCK ){
/* We cannot change ownership while we are holding a lock! */
return SQLITE_MISUSE;
}
TRACE4("Transfer ownership of %d from %d to %d\n", pFile->h,pFile->tid,hSelf);
pFile->tid = hSelf;
releaseLockInfo(pFile->pLock);
return findLockInfo(pFile->h, &pFile->pLock, 0);
rc = findLockInfo(pFile->h, &pFile->pLock, 0);
TRACE5("LOCK %d is now %s(%s,%d)\n", pFile->h,
locktypeName(pFile->locktype),
locktypeName(pFile->pLock->locktype), pFile->pLock->cnt);
return rc;
}
#else
# define transferOwnership(X) SQLITE_OK
@ -1121,24 +1146,6 @@ static int unixCheckReservedLock(OsFile *id){
return r;
}
#ifdef SQLITE_DEBUG
/*
** Helper function for printing out trace information from debugging
** binaries. This returns the string represetation of the supplied
** integer lock-type.
*/
static const char *locktypeName(int locktype){
switch( locktype ){
case NO_LOCK: return "NONE";
case SHARED_LOCK: return "SHARED";
case RESERVED_LOCK: return "RESERVED";
case PENDING_LOCK: return "PENDING";
case EXCLUSIVE_LOCK: return "EXCLUSIVE";
}
return "ERROR";
}
#endif
/*
** Lock the file with the lock specified by parameter locktype - one
** of the following:
@ -1240,6 +1247,7 @@ static int unixLock(OsFile *id, int locktype){
sqlite3OsLeaveMutex();
return rc;
}
pLock = pFile->pLock;
/* If some thread using this PID has a lock via a different OsFile*
** handle that precludes the requested lock, return BUSY.
@ -1439,7 +1447,7 @@ static int unixUnlock(OsFile *id, int locktype){
for(i=0; i<pOpen->nPending; i++){
close(pOpen->aPending[i]);
}
sqliteFree(pOpen->aPending);
free(pOpen->aPending);
pOpen->nPending = 0;
pOpen->aPending = 0;
}
@ -1458,12 +1466,11 @@ static int unixClose(OsFile **pId){
if( !id ) return SQLITE_OK;
rc = unixUnlock(*pId, NO_LOCK);
if( rc ) return rc;
if( id->dirfd>=0 ) close(id->dirfd);
id->dirfd = -1;
sqlite3OsEnterMutex();
if( id->pOpen->nLock ){
if( id->pOpen->nLock && rc==SQLITE_OK ){
/* If there are outstanding locks, do not actually close the file just
** yet because that would clear those locks. Instead, add the file
** descriptor to pOpen->aPending. It will be automatically closed when
@ -1471,7 +1478,7 @@ static int unixClose(OsFile **pId){
*/
int *aNew;
struct openCnt *pOpen = id->pOpen;
aNew = sqliteRealloc( pOpen->aPending, (pOpen->nPending+1)*sizeof(int) );
aNew = realloc( pOpen->aPending, (pOpen->nPending+1)*sizeof(int) );
if( aNew==0 ){
/* If a malloc fails, just leak the file descriptor */
}else{

View File

@ -13,7 +13,7 @@
** is not included in the SQLite library. It is used for automated
** testing of the SQLite library.
**
** $Id: test1.c,v 1.190 2006/01/15 00:13:16 drh Exp $
** $Id: test1.c,v 1.191 2006/01/15 02:30:58 drh Exp $
*/
#include "sqliteInt.h"
#include "tcl.h"
@ -166,7 +166,7 @@ static int getFilePointer(
** understood by scanf, and if not, try prepending an "0x" to see if
** that helps. If nothing works, a fatal error is generated.
*/
static int makePointerStr(Tcl_Interp *interp, char *zPtr, void *p){
int sqlite3TestMakePointerStr(Tcl_Interp *interp, char *zPtr, void *p){
sqlite3_snprintf(100, zPtr, "%p", p);
return TCL_OK;
}
@ -2096,7 +2096,7 @@ static int test_prepare(
}
if( pStmt ){
if( makePointerStr(interp, zBuf, pStmt) ) return TCL_ERROR;
if( sqlite3TestMakePointerStr(interp, zBuf, pStmt) ) return TCL_ERROR;
Tcl_AppendResult(interp, zBuf, 0);
}
return TCL_OK;
@ -2153,7 +2153,7 @@ static int test_prepare16(
Tcl_DecrRefCount(pTail);
if( pStmt ){
if( makePointerStr(interp, zBuf, pStmt) ) return TCL_ERROR;
if( sqlite3TestMakePointerStr(interp, zBuf, pStmt) ) return TCL_ERROR;
}
Tcl_AppendResult(interp, zBuf, 0);
#endif /* SQLITE_OMIT_UTF16 */
@ -2183,7 +2183,7 @@ static int test_open(
zFilename = Tcl_GetString(objv[1]);
rc = sqlite3_open(zFilename, &db);
if( makePointerStr(interp, zBuf, db) ) return TCL_ERROR;
if( sqlite3TestMakePointerStr(interp, zBuf, db) ) return TCL_ERROR;
Tcl_AppendResult(interp, zBuf, 0);
return TCL_OK;
}
@ -2212,7 +2212,7 @@ static int test_open16(
zFilename = Tcl_GetByteArrayFromObj(objv[1], 0);
rc = sqlite3_open16(zFilename, &db);
if( makePointerStr(interp, zBuf, db) ) return TCL_ERROR;
if( sqlite3TestMakePointerStr(interp, zBuf, db) ) return TCL_ERROR;
Tcl_AppendResult(interp, zBuf, 0);
#endif /* SQLITE_OMIT_UTF16 */
return TCL_OK;
@ -2606,7 +2606,7 @@ static int test_sqlite3OsOpenReadWrite(
Tcl_SetResult(interp, (char *)errorName(rc), TCL_STATIC);
return TCL_ERROR;
}
makePointerStr(interp, zBuf, pFile);
sqlite3TestMakePointerStr(interp, zBuf, pFile);
Tcl_SetResult(interp, zBuf, 0);
return TCL_ERROR;
}

View File

@ -11,7 +11,7 @@
*************************************************************************
** Code for testing the the SQLite library in a multithreaded environment.
**
** $Id: test4.c,v 1.14 2006/01/11 23:40:34 drh Exp $
** $Id: test4.c,v 1.15 2006/01/15 02:30:58 drh Exp $
*/
#include "sqliteInt.h"
#include "tcl.h"
@ -615,6 +615,73 @@ static int tcl_thread_swap(
return TCL_OK;
}
/*
** Usage: thread_db_get ID
**
** Return the database connection pointer for the given thread. Then
** remove the pointer from the thread itself. Afterwards, the thread
** can be stopped and the connection can be used by the main thread.
*/
static int tcl_thread_db_get(
void *NotUsed,
Tcl_Interp *interp, /* The TCL interpreter that invoked this command */
int argc, /* Number of arguments */
const char **argv /* Text of each argument */
){
int i;
char zBuf[100];
extern int sqlite3TestMakePointerStr(Tcl_Interp*, char*, void*);
if( argc!=2 ){
Tcl_AppendResult(interp, "wrong # args: should be \"", argv[0],
" ID", 0);
return TCL_ERROR;
}
i = parse_thread_id(interp, argv[1]);
if( i<0 ) return TCL_ERROR;
if( !threadset[i].busy ){
Tcl_AppendResult(interp, "no such thread", 0);
return TCL_ERROR;
}
thread_wait(&threadset[i]);
sqlite3TestMakePointerStr(interp, zBuf, threadset[i].db);
threadset[i].db = 0;
Tcl_SetResult(interp, zBuf, 0);
return TCL_OK;
}
/*
** Usage: thread_stmt_get ID
**
** Return the database stmt pointer for the given thread. Then
** remove the pointer from the thread itself.
*/
static int tcl_thread_stmt_get(
void *NotUsed,
Tcl_Interp *interp, /* The TCL interpreter that invoked this command */
int argc, /* Number of arguments */
const char **argv /* Text of each argument */
){
int i;
char zBuf[100];
extern int sqlite3TestMakePointerStr(Tcl_Interp*, char*, void*);
if( argc!=2 ){
Tcl_AppendResult(interp, "wrong # args: should be \"", argv[0],
" ID", 0);
return TCL_ERROR;
}
i = parse_thread_id(interp, argv[1]);
if( i<0 ) return TCL_ERROR;
if( !threadset[i].busy ){
Tcl_AppendResult(interp, "no such thread", 0);
return TCL_ERROR;
}
thread_wait(&threadset[i]);
sqlite3TestMakePointerStr(interp, zBuf, threadset[i].pStmt);
threadset[i].pStmt = 0;
Tcl_SetResult(interp, zBuf, 0);
return TCL_OK;
}
/*
** Register commands with the TCL interpreter.
*/
@ -635,6 +702,8 @@ int Sqlitetest4_Init(Tcl_Interp *interp){
{ "thread_step", (Tcl_CmdProc*)tcl_thread_step },
{ "thread_finalize", (Tcl_CmdProc*)tcl_thread_finalize },
{ "thread_swap", (Tcl_CmdProc*)tcl_thread_swap },
{ "thread_db_get", (Tcl_CmdProc*)tcl_thread_db_get },
{ "thread_stmt_get", (Tcl_CmdProc*)tcl_thread_stmt_get },
};
int i;

237
test/thread2.test Normal file
View File

@ -0,0 +1,237 @@
# 2006 January 14
#
# The author disclaims copyright to this source code. In place of
# a legal notice, here is a blessing:
#
# May you do good and not evil.
# May you find forgiveness for yourself and forgive others.
# May you share freely, never taking more than you give.
#
#***********************************************************************
# This file implements regression tests for SQLite library. The
# focus of this script is multithreading behavior
#
# $Id: thread2.test,v 1.1 2006/01/15 02:30:58 drh Exp $
set testdir [file dirname $argv0]
source $testdir/tester.tcl
# Skip this whole file if the thread testing code is not enabled
#
if {[llength [info command thread_step]]==0 || [sqlite3 -has-codec]} {
finish_test
return
}
if {![info exists threadsOverrideEachOthersLocks]} {
finish_test
return
}
# Create some data to work with
#
do_test thread1-1.1 {
execsql {
CREATE TABLE t1(a,b);
INSERT INTO t1 VALUES(1,'abcdefgh');
INSERT INTO t1 SELECT a+1, b||b FROM t1;
INSERT INTO t1 SELECT a+2, b||b FROM t1;
INSERT INTO t1 SELECT a+4, b||b FROM t1;
SELECT count(*), max(length(b)) FROM t1;
}
} {8 64}
# Use the thread_swap command to move the database connections between
# threads, then verify that they still work.
#
do_test thread2-1.2 {
db close
thread_create A test.db
thread_create B test.db
thread_swap A B
thread_compile A {SELECT a FROM t1 LIMIT 1}
thread_result A
} {SQLITE_OK}
do_test thread2-1.3 {
thread_step A
thread_result A
} {SQLITE_ROW}
do_test thread2-1.4 {
thread_argv A 0
} {1}
do_test thread2-1.5 {
thread_finalize A
thread_result A
} {SQLITE_OK}
do_test thread2-1.6 {
thread_compile B {SELECT a FROM t1 LIMIT 1}
thread_result B
} {SQLITE_OK}
do_test thread2-1.7 {
thread_step B
thread_result B
} {SQLITE_ROW}
do_test thread2-1.8 {
thread_argv B 0
} {1}
do_test thread2-1.9 {
thread_finalize B
thread_result B
} {SQLITE_OK}
# Swap them again.
#
do_test thread2-2.2 {
thread_swap A B
thread_compile A {SELECT a FROM t1 LIMIT 1}
thread_result A
} {SQLITE_OK}
do_test thread2-2.3 {
thread_step A
thread_result A
} {SQLITE_ROW}
do_test thread2-2.4 {
thread_argv A 0
} {1}
do_test thread2-2.5 {
thread_finalize A
thread_result A
} {SQLITE_OK}
do_test thread2-2.6 {
thread_compile B {SELECT a FROM t1 LIMIT 1}
thread_result B
} {SQLITE_OK}
do_test thread2-2.7 {
thread_step B
thread_result B
} {SQLITE_ROW}
do_test thread2-2.8 {
thread_argv B 0
} {1}
do_test thread2-2.9 {
thread_finalize B
thread_result B
} {SQLITE_OK}
thread_halt A
thread_halt B
# Save the original (correct) value of threadsOverrideEachOthersLocks
# so that it can be restored. If this value is left set incorrectly, lots
# of things will go wrong in future tests.
#
set orig_threadOverride $threadsOverrideEachOthersLocks
# Pretend we are on a system (like RedHat9) were threads do not
# override each others locks.
#
set threadsOverrideEachOthersLocks 0
# Verify that we can move database connections between threads as
# long as no locks are held.
#
do_test thread2-3.1 {
thread_create A test.db
set DB [thread_db_get A]
thread_halt A
} {}
do_test thread2-3.2 {
set STMT [sqlite3_prepare $DB {SELECT a FROM t1 LIMIT 1} -1 TAIL]
sqlite3_step $STMT
} SQLITE_ROW
do_test thread2-3.3 {
sqlite3_column_int $STMT 0
} 1
do_test thread2-3.4 {
sqlite3_finalize $STMT
} SQLITE_OK
do_test thread2-3.5 {
set STMT [sqlite3_prepare $DB {SELECT max(a) FROM t1} -1 TAIL]
sqlite3_step $STMT
} SQLITE_ROW
do_test thread2-3.6 {
sqlite3_column_int $STMT 0
} 8
do_test thread2-3.7 {
sqlite3_finalize $STMT
} SQLITE_OK
do_test thread2-3.8 {
sqlite3_close $DB
} {SQLITE_OK}
do_test thread2-3.10 {
thread_create A test.db
thread_compile A {SELECT a FROM t1 LIMIT 1}
thread_step A
thread_finalize A
set DB [thread_db_get A]
thread_halt A
} {}
do_test thread2-3.11 {
set STMT [sqlite3_prepare $DB {SELECT a FROM t1 LIMIT 1} -1 TAIL]
sqlite3_step $STMT
} SQLITE_ROW
do_test thread2-3.12 {
sqlite3_column_int $STMT 0
} 1
do_test thread2-3.13 {
sqlite3_finalize $STMT
} SQLITE_OK
do_test thread2-3.14 {
sqlite3_close $DB
} SQLITE_OK
do_test thread2-3.20 {
thread_create A test.db
thread_compile A {SELECT a FROM t1 LIMIT 3}
thread_step A
set STMT [thread_stmt_get A]
set DB [thread_db_get A]
thread_halt A
} {}
do_test thread2-3.21 {
sqlite3_step $STMT
} SQLITE_ROW
do_test thread2-3.22 {
sqlite3_column_int $STMT 0
} 2
do_test thread2-3.23 {
# The unlock fails here. But because we never check the return
# code from sqlite3OsUnlock (because we cannot do anything about it
# if it fails) we do not realize that an error has occurred.
sqlite3_finalize $STMT
} SQLITE_OK
do_test thread2-3.25 {
sqlite3_close $DB
} SQLITE_OK
do_test thread2-3.30 {
thread_create A test.db
thread_compile A {BEGIN}
thread_step A
thread_finalize A
thread_compile A {SELECT a FROM t1 LIMIT 1}
thread_step A
thread_finalize A
set DB [thread_db_get A]
thread_halt A
} {}
do_test thread2-3.31 {
set STMT [sqlite3_prepare $DB {INSERT INTO t1 VALUES(99,'error')} -1 TAIL]
sqlite3_step $STMT
} SQLITE_ERROR
do_test thread2-3.32 {
sqlite3_finalize $STMT
} SQLITE_MISUSE
do_test thread2-3.33 {
sqlite3_close $DB
} SQLITE_OK
# VERY important to set the override flag back to its true value.
#
set threadsOverrideEachOthersLocks $orig_threadOverride
# Also important to halt the worker threads, which are using spin
# locks and eating away CPU cycles.
#
thread_halt *
finish_test