From 87ddfeb079a3d67cbfa1334d9cce5b31263f3d13 Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 27 May 2013 10:11:53 +0000 Subject: [PATCH] When writing to an FTS table, take an exclusive shared-cache lock on the %_segdir table before writing. Otherwise, an xCommit() call may report an SQLITE_LOCKED error. FossilOrigin-Name: 3cd2da42e9403b1e6243ad53f3f2bbf89c0fb9b0 --- ext/fts3/fts3.c | 4 -- ext/fts3/fts3_write.c | 44 ++++++++---------- manifest | 20 ++++---- manifest.uuid | 2 +- test/fts3shared.test | 105 ++++++++++++++++++++++++++++++++++++++++++ test/trace2.test | 1 + 6 files changed, 137 insertions(+), 39 deletions(-) diff --git a/ext/fts3/fts3.c b/ext/fts3/fts3.c index c00a13f5a1..374d690688 100644 --- a/ext/fts3/fts3.c +++ b/ext/fts3/fts3.c @@ -2984,11 +2984,7 @@ static int fts3FilterMethod( return rc; } - rc = sqlite3Fts3ReadLock(p); - if( rc!=SQLITE_OK ) return rc; - rc = fts3EvalStart(pCsr); - sqlite3Fts3SegmentsClose(p); if( rc!=SQLITE_OK ) return rc; pCsr->pNextId = pCsr->aDoclist; diff --git a/ext/fts3/fts3_write.c b/ext/fts3/fts3_write.c index 269d1dd599..2ee0e3d5fa 100644 --- a/ext/fts3/fts3_write.c +++ b/ext/fts3/fts3_write.c @@ -489,37 +489,30 @@ static void fts3SqlExec( /* -** This function ensures that the caller has obtained a shared-cache -** table-lock on the %_content table. This is required before reading -** data from the fts3 table. If this lock is not acquired first, then -** the caller may end up holding read-locks on the %_segments and %_segdir -** tables, but no read-lock on the %_content table. If this happens -** a second connection will be able to write to the fts3 table, but -** attempting to commit those writes might return SQLITE_LOCKED or -** SQLITE_LOCKED_SHAREDCACHE (because the commit attempts to obtain -** write-locks on the %_segments and %_segdir ** tables). +** This function ensures that the caller has obtained an exclusive +** shared-cache table-lock on the %_segdir table. This is required before +** writing data to the fts3 table. If this lock is not acquired first, then +** the caller may end up attempting to take this lock as part of committing +** a transaction, causing SQLite to return SQLITE_LOCKED or +** LOCKED_SHAREDCACHEto a COMMIT command. ** -** We try to avoid this because if FTS3 returns any error when committing -** a transaction, the whole transaction will be rolled back. And this is -** not what users expect when they get SQLITE_LOCKED_SHAREDCACHE. It can -** still happen if the user reads data directly from the %_segments or -** %_segdir tables instead of going through FTS3 though. -** -** This reasoning does not apply to a content=xxx table. +** It is best to avoid this because if FTS3 returns any error when +** committing a transaction, the whole transaction will be rolled back. +** And this is not what users expect when they get SQLITE_LOCKED_SHAREDCACHE. +** It can still happen if the user locks the underlying tables directly +** instead of accessing them via FTS. */ -int sqlite3Fts3ReadLock(Fts3Table *p){ - int rc; /* Return code */ - sqlite3_stmt *pStmt; /* Statement used to obtain lock */ - - if( p->zContentTbl==0 ){ - rc = fts3SqlStmt(p, SQL_SELECT_CONTENT_BY_ROWID, &pStmt, 0); +static int fts3Writelock(Fts3Table *p){ + int rc = SQLITE_OK; + + if( p->nPendingData==0 ){ + sqlite3_stmt *pStmt; + rc = fts3SqlStmt(p, SQL_DELETE_SEGDIR_LEVEL, &pStmt, 0); if( rc==SQLITE_OK ){ sqlite3_bind_null(pStmt, 1); sqlite3_step(pStmt); rc = sqlite3_reset(pStmt); } - }else{ - rc = SQLITE_OK; } return rc; @@ -5297,6 +5290,9 @@ int sqlite3Fts3UpdateMethod( aSzIns = &aSzDel[p->nColumn+1]; memset(aSzDel, 0, sizeof(aSzDel[0])*(p->nColumn+1)*2); + rc = fts3Writelock(p); + if( rc!=SQLITE_OK ) goto update_out; + /* If this is an INSERT operation, or an UPDATE that modifies the rowid ** value, then this operation requires constraint handling. ** diff --git a/manifest b/manifest index cf297405cb..29a52f2578 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Set\s_XOPEN_SOURCE\sto\s600\sso\sthat\sfchmod()\swill\s(hopefully)\swork\son\sFreeBSD. -D 2013-05-24T12:47:26.703 +C When\swriting\sto\san\sFTS\stable,\stake\san\sexclusive\sshared-cache\slock\son\sthe\s%_segdir\stable\sbefore\swriting.\sOtherwise,\san\sxCommit()\scall\smay\sreport\san\sSQLITE_LOCKED\serror. +D 2013-05-27T10:11:53.308 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in f6b58b7bdf6535f0f0620c486dd59aa4662c0b4f F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -55,7 +55,7 @@ F ext/fts3/README.content fdc666a70d5257a64fee209f97cf89e0e6e32b51 F ext/fts3/README.syntax a19711dc5458c20734b8e485e75fb1981ec2427a F ext/fts3/README.tokenizers e0a8b81383ea60d0334d274fadf305ea14a8c314 F ext/fts3/README.txt 8c18f41574404623b76917b9da66fcb0ab38328d -F ext/fts3/fts3.c 4bc160e6ff9ab5456b600f389f8941485ea5082f +F ext/fts3/fts3.c 931b3c83abdd1ab3bb389b2130431c2a9ff73b91 F ext/fts3/fts3.h 3a10a0af180d502cecc50df77b1b22df142817fe F ext/fts3/fts3Int.h 0b167bed9e63151635620a4f639bc62ac6012cba F ext/fts3/fts3_aux.c b02632f6dd0e375ce97870206d914ea6d8df5ccd @@ -73,7 +73,7 @@ F ext/fts3/fts3_tokenizer.h 64c6ef6c5272c51ebe60fc607a896e84288fcbc3 F ext/fts3/fts3_tokenizer1.c 5c98225a53705e5ee34824087478cf477bdb7004 F ext/fts3/fts3_unicode.c 92391b4b4fb043564c6539ea9b8661e3bcba47b9 F ext/fts3/fts3_unicode2.c a863f05f758af36777dffc2facc898bc73fec896 -F ext/fts3/fts3_write.c d92c6cbe07363791cfe8b62b4dee67e6f8afc9e2 +F ext/fts3/fts3_write.c 6a1fc0e922e76b68e594bf7bc33bac72af9dc47b F ext/fts3/fts3speed.tcl b54caf6a18d38174f1a6e84219950d85e98bb1e9 F ext/fts3/mkfts3amal.tcl 252ecb7fe6467854f2aa237bf2c390b74e71f100 F ext/fts3/tool/fts3view.c 6cfc5b67a5f0e09c0d698f9fd012c784bfaa9197 @@ -513,7 +513,7 @@ F test/fts3prefix.test b36d4f00b128a51e7b386cc013a874246d9d7dc1 F test/fts3prefix2.test 477ca96e67f60745b7ac931cfa6e9b080c562da5 F test/fts3query.test ef79d31fdb355d094baec1c1b24b60439a1fb8a2 F test/fts3rnd.test 1320d8826a845e38a96e769562bf83d7a92a15d0 -F test/fts3shared.test 8bb266521d7c5495c0ae522bb4d376ad5387d4a2 +F test/fts3shared.test c2f60e152e8554549eb25f0a7593ea01389c5037 F test/fts3snippet.test 8e956051221a34c7daeb504f023cb54d5fa5a8b2 F test/fts3sort.test 95be0b19d7e41c44b29014f13ea8bddd495fd659 F test/fts3tok1.test 4d9e7401679dc71f6b2f76416309b923210bfdbe @@ -924,7 +924,7 @@ F test/tkt3997.test a335fa41ca3985660a139df7b734a26ef53284bd F test/tkt4018.test 7c2c9ba4df489c676a0a7a0e809a1fb9b2185bd1 F test/tokenize.test ce430a7aed48fc98301611429595883fdfcab5d7 F test/trace.test 4b36a41a3e9c7842151af6da5998f5080cdad9e5 -F test/trace2.test c1dc104a8d11a347c870cfea6235e3fc6f6cb06d +F test/trace2.test e7a988fdd982cdec62f1f1f34b0360e6476d01a0 F test/trans.test 6e1b4c6a42dba31bd65f8fa5e61a2708e08ddde6 F test/trans2.test d5337e61de45e66b1fcbf9db833fa8c82e624b22 F test/trans3.test 373ac5183cc56be69f48ae44090e7f672939f732 @@ -1066,7 +1066,7 @@ F tool/vdbe-compress.tcl f12c884766bd14277f4fcedcae07078011717381 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh fbc018d67fd7395f440c28f33ef0f94420226381 F tool/win/sqlite.vsix 97894c2790eda7b5bce3cc79cb2a8ec2fde9b3ac -P 9e2c17c5358b156b588542dbba38da7fedf5302b -R 447bfb9d68d03e84058ef9e235e367ce -U drh -Z 0a3675dfa27f37f077377df6c72c7598 +P 61a10452399db28cd5ea4ba9d416b87a34c2eddb +R 5b2b7d523083ab3d77d86c0cce4d267d +U dan +Z 5b590f475035536f2f371b845c79a1e4 diff --git a/manifest.uuid b/manifest.uuid index d490615225..cd1334fa37 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -61a10452399db28cd5ea4ba9d416b87a34c2eddb \ No newline at end of file +3cd2da42e9403b1e6243ad53f3f2bbf89c0fb9b0 \ No newline at end of file diff --git a/test/fts3shared.test b/test/fts3shared.test index 5f75bd5fe6..a587a437bf 100644 --- a/test/fts3shared.test +++ b/test/fts3shared.test @@ -1,3 +1,4 @@ +# # 2010 September 17 # # May you do good and not evil. @@ -5,6 +6,9 @@ # May you share freely, never taking more than you give. # #*********************************************************************** +# This file implements regression tests for SQLite library. The +# focus of this file is the interactions between the FTS3/4 module +# and shared-cache mode. # set testdir [file dirname $argv0] @@ -14,6 +18,7 @@ ifcapable !fts3||!shared_cache { finish_test return } +set ::testprefix fts3shared db close set ::enable_shared_cache [sqlite3_enable_shared_cache 1] @@ -67,6 +72,106 @@ do_test fts3shared-1.6 { sqlite3_get_autocommit db2 } 0 db close db2 close +#------------------------------------------------------------------------- +# The following tests - fts3shared-2.* - test that unless FTS is bypassed +# and the underlying tables accessed directly, it is not possible for an +# SQLITE_LOCKED error to be enountered when committing an FTS transaction. +# +# Any SQLITE_LOCKED error should be returned when the fts4 (or fts4aux) +# table is first read/written within a transaction, not later on. +# +set LOCKED {1 {database table is locked}} +forcedelete test.db +sqlite3 dbR test.db +sqlite3 dbW test.db +do_test 2.1 { + execsql { + CREATE VIRTUAL TABLE t1 USING fts4; + CREATE TABLE t2ext(a, b); + CREATE VIRTUAL TABLE t2 USING fts4(content=t2ext); + CREATE VIRTUAL TABLE t1aux USING fts4aux(t1); + CREATE VIRTUAL TABLE t2aux USING fts4aux(t2); + + INSERT INTO t1 VALUES('a b c'); + INSERT INTO t2(rowid, a, b) VALUES(1, 'd e f', 'g h i'); + } dbW +} {} + +# Test that once [dbW] has written to the FTS table, no client may read +# from the FTS or fts4aux table. +do_test 2.2.1 { + execsql { + BEGIN; + INSERT INTO t1 VALUES('j k l'); + } dbW + execsql BEGIN dbR +} {} +do_test 2.2.2 { catchsql "SELECT * FROM t1 WHERE rowid=1" dbR } $LOCKED +do_test 2.2.3 { catchsql "SELECT * FROM t1 WHERE t1 MATCH 'a'" dbR } $LOCKED +do_test 2.2.4 { catchsql "SELECT rowid FROM t1 WHERE t1 MATCH 'a'" dbR } $LOCKED +do_test 2.2.5 { catchsql "SELECT * FROM t1" dbR } $LOCKED +do_test 2.2.6 { catchsql "SELECT * FROM t1aux" dbR } $LOCKED +do_test 2.2.7 { execsql COMMIT dbW } {} +do_test 2.2.8 { execsql COMMIT dbR } {} + +# Same test as 2.2.*, except with a content= table. +# +do_test 2.3.1 { + execsql { + BEGIN; + INSERT INTO t2(rowid, a, b) VALUES(2, 'j k l', 'm n o'); + } dbW + execsql BEGIN dbR +} {} +do_test 2.3.3 { catchsql "SELECT * FROM t2 WHERE t2 MATCH 'a'" dbR } $LOCKED +do_test 2.3.4 { catchsql "SELECT rowid FROM t2 WHERE t2 MATCH 'a'" dbR } $LOCKED +do_test 2.3.6 { catchsql "SELECT * FROM t2aux" dbR } $LOCKED +do_test 2.3.7 { execsql COMMIT dbW } {} +do_test 2.3.8 { execsql COMMIT dbR } {} + +# Test that once a connection has read from the FTS or fts4aux table, +# another connection may not write to the FTS table. +# +foreach {tn sql} { + 1 "SELECT * FROM t1 WHERE rowid=1" + 2 "SELECT * FROM t1 WHERE t1 MATCH 'a'" + 3 "SELECT rowid FROM t1 WHERE t1 MATCH 'a'" + 4 "SELECT * FROM t1" + 5 "SELECT * FROM t1aux" +} { + + do_test 2.4.$tn { + execsql BEGIN dbR + execsql $::sql dbR + execsql BEGIN dbW + catchsql "INSERT INTO t1 VALUES('p q r')" dbW + } $LOCKED + + execsql ROLLBACK dbR + execsql ROLLBACK dbW +} + +# Same test as 2.4.*, except with a content= table. +# +foreach {tn sql} { + 2 "SELECT * FROM t2 WHERE t2 MATCH 'a'" + 3 "SELECT rowid FROM t2 WHERE t2 MATCH 'a'" + 5 "SELECT * FROM t2aux" +} { + + do_test 2.5.$tn { + execsql BEGIN dbR + execsql $::sql dbR + execsql BEGIN dbW + catchsql "INSERT INTO t2(rowid, a, b) VALUES(3, 's t u', 'v w x')" dbW + } $LOCKED + + execsql ROLLBACK dbR + execsql ROLLBACK dbW +} + +dbW close +dbR close sqlite3_enable_shared_cache $::enable_shared_cache finish_test diff --git a/test/trace2.test b/test/trace2.test index 2f7ae7d5a2..8f68d87585 100644 --- a/test/trace2.test +++ b/test/trace2.test @@ -128,6 +128,7 @@ ifcapable fts3 { INSERT INTO x1 VALUES('North northwest wind between 8 and 14 mph'); } { "INSERT INTO x1 VALUES('North northwest wind between 8 and 14 mph');" + "-- DELETE FROM 'main'.'x1_segdir' WHERE level = ?" "-- INSERT INTO 'main'.'x1_content' VALUES(?,(?))" "-- REPLACE INTO 'main'.'x1_docsize' VALUES(?,?)" "-- SELECT value FROM 'main'.'x1_stat' WHERE id=?" -- 2.47.2