Update sqlite3_snapshot_open() to reduce the chances of reading a corrupt snapshot created by a checkpointer process exiting unexpectedly.
FossilOrigin-Name: 7315f7cbf4179aadda0f1a0baa1526a9b9f9729f
This commit is contained in:
dan
parent
ee3b7a2767
commit
65127cd57d
18
manifest
18
manifest
@ -1,5 +1,5 @@
|
||||
C Merge\sunrelated\sfixes\sfrom\strunk.
|
||||
D 2015-12-09T16:04:06.348
|
||||
C Update\ssqlite3_snapshot_open()\sto\sreduce\sthe\schances\sof\sreading\sa\scorrupt\ssnapshot\screated\sby\sa\scheckpointer\sprocess\sexiting\sunexpectedly.
|
||||
D 2015-12-09T20:05:27.534
|
||||
F Makefile.in 28bcd6149e050dff35d4dcfd97e890cd387a499d
|
||||
F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434
|
||||
F Makefile.msc e8fdca1cb89a1b58b5f4d3a130ea9a3d28cb314d
|
||||
@ -341,7 +341,7 @@ F src/resolve.c a83b41104e6ff69855d03cd0aaa09e93927ec39f
|
||||
F src/rowset.c eccf6af6d620aaa4579bd3b72c1b6395d9e9fa1e
|
||||
F src/select.c f8fded11fc443a9f5a73cc5db069d06b34460e2f
|
||||
F src/shell.c abbc74ea43dbf2f306ea18282d666683fb5efab2
|
||||
F src/sqlite.h.in fc8a2875a318df1b9dabd82cb00b1ac98081423a
|
||||
F src/sqlite.h.in 19dea4862ccfcc1a733d0fd18d4744b02a505ac6
|
||||
F src/sqlite3.rc 992c9f5fb8285ae285d6be28240a7e8d3a7f2bad
|
||||
F src/sqlite3ext.h dfbe62ffd95b99afe2140d8c35b180d11924072d
|
||||
F src/sqliteInt.h 5caacf37a776f9d6178e519cb0b5248ca22a3828
|
||||
@ -415,7 +415,7 @@ F src/vdbesort.c a7ec02da4494c59dfd071126dd3726be5a11459d
|
||||
F src/vdbetrace.c 8befe829faff6d9e6f6e4dee5a7d3f85cc85f1a0
|
||||
F src/vtab.c 2a8b44aa372c33f6154208e7a7f6c44254549806
|
||||
F src/vxworks.h c18586c8edc1bddbc15c004fa16aeb1e1342b4fb
|
||||
F src/wal.c abce669053edf5cd1cd1751d654d48d74ed47839
|
||||
F src/wal.c 0bd8aa8e0db924493af4c72f527afc9b9e22257a
|
||||
F src/wal.h 907943dfdef10b583e81906679a347e0ec6f1b1b
|
||||
F src/walker.c 2e14d17f592d176b6dc879c33fbdec4fbccaa2ba
|
||||
F src/where.c b18edbb9e5afabb77f4f27550c471c5c824e0fe7
|
||||
@ -1020,7 +1020,7 @@ F test/skipscan2.test d1d1450952b7275f0b0a3a981f0230532743951a
|
||||
F test/skipscan3.test ec5bab3f81c7038b43450e7b3062e04a198bdbb5
|
||||
F test/skipscan5.test 67817a4b6857c47e0e33ba3e506da6f23ef68de2
|
||||
F test/skipscan6.test 5866039d03a56f5bd0b3d172a012074a1d90a15b
|
||||
F test/snapshot.test 061dc75b77ca65c0e9c5976499625abe5be7a5c0
|
||||
F test/snapshot.test 800e0be4488acb88dd38ff9e9b83edb71d9d5a9d
|
||||
F test/soak.test 0b5b6375c9f4110c828070b826b3b4b0bb65cd5f
|
||||
F test/softheap1.test 843cd84db9891b2d01b9ab64cef3e9020f98d087
|
||||
F test/sort.test 3f492e5b7be1d3f756728d2ff6edf4f6091e84cb
|
||||
@ -1409,7 +1409,7 @@ F tool/vdbe_profile.tcl 246d0da094856d72d2c12efec03250d71639d19f
|
||||
F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
|
||||
F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b
|
||||
F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
|
||||
P 502cc6f353358946080d9bcd335aed526825b88a 901d0b8f3b72e96ffa8e9436993a12980f5ebd51
|
||||
R 9b1d7228446872f8d72f41cfd12e515e
|
||||
U drh
|
||||
Z 3ae5744205bfcf5cc63a2630633fbd47
|
||||
P 362615b4df94358d0264b0991c3090a0878f054c
|
||||
R cfc950fbd468350b7777662be2b401a3
|
||||
U dan
|
||||
Z 51de2f0ad57bbe117dc864890e31b83c
|
||||
|
||||
@ -1 +1 @@
|
||||
362615b4df94358d0264b0991c3090a0878f054c
|
||||
7315f7cbf4179aadda0f1a0baa1526a9b9f9729f
|
||||
@ -7895,11 +7895,39 @@ int sqlite3_db_cacheflush(sqlite3*);
|
||||
** SQLITE_ERROR is returned. If any other error occurs, for example an IO
|
||||
** error or an OOM condition, the corresponding SQLite error code is
|
||||
** returned.
|
||||
**
|
||||
** Each successful call to sqlite3_snapshot_get() must be matched by a call
|
||||
** to sqlite3_snapshot_free() to delete the snapshot handle. Not doing so
|
||||
** is a memory leak. The results of using a snapshot handle after it has
|
||||
** been deleted by sqlite3_snapshot_free() are undefined.
|
||||
**
|
||||
** Given a snapshot handle, the sqlite3_snapshot_open() API function may be
|
||||
** used to open a read transaction on the same database snapshot that was
|
||||
** being read when sqlite3_snapshot_get() was called to obtain it. The
|
||||
** combination of the first two arguments to sqlite3_snapshot_open() - a
|
||||
** database handle and the name (e.g. "main") of one of its attached
|
||||
** databases - must refer to the same database file as that identified by
|
||||
** the arguments passed to the sqlite3_snapshot_get() call. The database
|
||||
** handle must not have an open read or write transaction on this database
|
||||
** file, and must not be in auto-commit mode.
|
||||
**
|
||||
** An old database snapshot may only be opened if SQLite is able to
|
||||
** determine that it is still valid. The only way for an application to
|
||||
** guarantee that a snapshot remains valid is by holding an open
|
||||
** read-transaction on it or on an older snapshot of the same database
|
||||
** file. If SQLite cannot determine that the snapshot identified by the
|
||||
** snapshot handle, SQLITE_BUSY_SNAPSHOT is returned.
|
||||
**
|
||||
** Otherwise, if the read transaction is successfully opened, SQLITE_OK is
|
||||
** returned. If the named database is not in wal mode or if the database
|
||||
** handle already has an open read or write transaction on it, or if the
|
||||
** database handle is in auto-commit mode, SQLITE_ERROR is returned. If
|
||||
** an OOM or IO error occurs, the associated SQLite error code is returned.
|
||||
*/
|
||||
typedef struct sqlite3_snapshot sqlite3_snapshot;
|
||||
int sqlite3_snapshot_get(sqlite3*, const char*, sqlite3_snapshot **ppSnapshot);
|
||||
int sqlite3_snapshot_open(sqlite3*, const char*, sqlite3_snapshot*);
|
||||
void sqlite3_snapshot_free(sqlite3_snapshot*);
|
||||
int sqlite3_snapshot_open(sqlite3*, const char*, sqlite3_snapshot*);
|
||||
|
||||
/*
|
||||
** Undo the hack that converts floating point types to integer for
|
||||
|
||||
66
src/wal.c
66
src/wal.c
@ -2275,6 +2275,9 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
|
||||
{
|
||||
if( (pWal->readOnly & WAL_SHM_RDONLY)==0
|
||||
&& (mxReadMark<mxFrame || mxI==0)
|
||||
#ifdef SQLITE_ENABLE_SNAPSHOT
|
||||
&& pWal->pSnapshot==0
|
||||
#endif
|
||||
){
|
||||
for(i=1; i<WAL_NREADER; i++){
|
||||
rc = walLockExclusive(pWal, WAL_READ_LOCK(i), 1);
|
||||
@ -2289,6 +2292,9 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
|
||||
}
|
||||
}
|
||||
if( mxI==0 ){
|
||||
#ifdef SQLITE_ENABLE_SNAPSHOT
|
||||
if( pWal->pSnapshot ) return SQLITE_BUSY_SNAPSHOT;
|
||||
#endif
|
||||
assert( rc==SQLITE_BUSY || (pWal->readOnly & WAL_SHM_RDONLY)!=0 );
|
||||
return rc==SQLITE_BUSY ? WAL_RETRY : SQLITE_READONLY_CANTLOCK;
|
||||
}
|
||||
@ -2383,22 +2389,54 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){
|
||||
#ifdef SQLITE_ENABLE_SNAPSHOT
|
||||
if( rc==SQLITE_OK ){
|
||||
if( pSnapshot && memcmp(pSnapshot, &pWal->hdr, sizeof(WalIndexHdr)) ){
|
||||
/* At this point the client has a lock on an aReadMark[] slot holding
|
||||
** a value equal to or smaller than pSnapshot->mxFrame. This client
|
||||
** did not populate the aReadMark[] slot. pWal->hdr is populated with
|
||||
** the wal-index header for the snapshot currently at the head of the
|
||||
** wal file, which is different from pSnapshot.
|
||||
**
|
||||
** The presence of the aReadMark[] slot entry makes it very likely
|
||||
** that either there is currently another read-transaction open on
|
||||
** pSnapshot, or that there has been one more recently than the last
|
||||
** checkpoint of any frames greater than pSnapshot->mxFrame was
|
||||
** started. There is an exception though: client 1 may have called
|
||||
** walTryBeginRead and started to open snapshot pSnapshot, setting
|
||||
** the aReadMark[] slot to do so. At the same time, client 2 may
|
||||
** have committed a new snapshot to disk and started a checkpoint.
|
||||
** In this circumstance client 1 does not end up reading pSnapshot,
|
||||
** but may leave the aReadMark[] slot populated.
|
||||
**
|
||||
** The race condition above is difficult to detect. One approach would
|
||||
** be to check the aReadMark[] slot for another client. But this is
|
||||
** prone to false-positives from other snapshot clients. And there
|
||||
** is no equivalent to xCheckReservedLock() for wal locks. Another
|
||||
** approach would be to take the checkpointer lock and check that
|
||||
** fewer than pSnapshot->mxFrame frames have been checkpointed. But
|
||||
** that does not account for checkpointer processes that failed after
|
||||
** checkpointing frames but before updating WalCkptInfo.nBackfill.
|
||||
** And it would mean that this function would block on checkpointers
|
||||
** and vice versa.
|
||||
**
|
||||
** TODO: For now, this race condition is ignored.
|
||||
*/
|
||||
volatile WalCkptInfo *pInfo = walCkptInfo(pWal);
|
||||
rc = walLockShared(pWal, WAL_READ_LOCK(0));
|
||||
if( rc==SQLITE_OK ){
|
||||
if( pInfo->nBackfill<=pSnapshot->mxFrame
|
||||
&& pSnapshot->aSalt[0]==pWal->hdr.aSalt[0]
|
||||
&& pSnapshot->aSalt[1]==pWal->hdr.aSalt[1]
|
||||
){
|
||||
assert( pWal->readLock>0 );
|
||||
assert( pInfo->aReadMark[pWal->readLock]<=pSnapshot->mxFrame );
|
||||
memcpy(&pWal->hdr, pSnapshot, sizeof(WalIndexHdr));
|
||||
*pChanged = bChanged;
|
||||
}else{
|
||||
rc = SQLITE_BUSY_SNAPSHOT;
|
||||
}
|
||||
walUnlockShared(pWal, WAL_READ_LOCK(0));
|
||||
|
||||
assert( pWal->readLock>0 );
|
||||
assert( pInfo->aReadMark[pWal->readLock]<=pSnapshot->mxFrame );
|
||||
|
||||
/* Check that the wal file has not been wrapped. Assuming it has not,
|
||||
** overwrite pWal->hdr with *pSnapshot and set *pChanged as appropriate
|
||||
** for opening the snapshot. Or, if the wal file has been wrapped
|
||||
** since pSnapshot was written, return SQLITE_BUSY_SNAPSHOT. */
|
||||
if( pSnapshot->aSalt[0]==pWal->hdr.aSalt[0]
|
||||
&& pSnapshot->aSalt[1]==pWal->hdr.aSalt[1]
|
||||
){
|
||||
memcpy(&pWal->hdr, pSnapshot, sizeof(WalIndexHdr));
|
||||
*pChanged = bChanged;
|
||||
}else{
|
||||
rc = SQLITE_BUSY_SNAPSHOT;
|
||||
}
|
||||
|
||||
if( rc!=SQLITE_OK ){
|
||||
sqlite3WalEndReadTransaction(pWal);
|
||||
}
|
||||
|
||||
@ -57,12 +57,14 @@ do_execsql_test 1.3.2 COMMIT
|
||||
# Check that a simple case works. Reuse the database created by the
|
||||
# block of tests above.
|
||||
#
|
||||
do_execsql_test 2.0 {
|
||||
# UPDATE: This case (2.1) no longer works. 2.2 does.
|
||||
#
|
||||
do_execsql_test 2.1.0 {
|
||||
BEGIN;
|
||||
SELECT * FROM t1;
|
||||
} {1 2 3 4 5 6 7 8}
|
||||
|
||||
do_test 2.1 {
|
||||
do_test 2.1.1 {
|
||||
set snapshot [sqlite3_snapshot_get db main]
|
||||
execsql {
|
||||
COMMIT;
|
||||
@ -71,17 +73,47 @@ do_test 2.1 {
|
||||
}
|
||||
} {1 2 3 4 5 6 7 8 9 10}
|
||||
|
||||
do_test 2.2 {
|
||||
do_test 2.1.2 {
|
||||
execsql BEGIN
|
||||
sqlite3_snapshot_open db main $snapshot
|
||||
execsql { SELECT * FROM t1 }
|
||||
} {1 2 3 4 5 6 7 8}
|
||||
list [catch { sqlite3_snapshot_open db main $snapshot } msg] $msg
|
||||
} {1 SQLITE_BUSY_SNAPSHOT}
|
||||
|
||||
do_test 2.3 {
|
||||
do_test 2.1.3 {
|
||||
sqlite3_snapshot_free $snapshot
|
||||
execsql COMMIT
|
||||
} {}
|
||||
|
||||
do_test 2.2.0 {
|
||||
sqlite3 db2 test.db
|
||||
execsql {
|
||||
BEGIN;
|
||||
SELECT * FROM t1;
|
||||
} db2
|
||||
} {1 2 3 4 5 6 7 8 9 10}
|
||||
|
||||
do_test 2.2.1 {
|
||||
set snapshot [sqlite3_snapshot_get db2 main]
|
||||
execsql {
|
||||
INSERT INTO t1 VALUES(11, 12);
|
||||
SELECT * FROM t1;
|
||||
}
|
||||
} {1 2 3 4 5 6 7 8 9 10 11 12}
|
||||
|
||||
do_test 2.1.2 {
|
||||
execsql BEGIN
|
||||
sqlite3_snapshot_open db main $snapshot
|
||||
execsql {
|
||||
SELECT * FROM t1;
|
||||
}
|
||||
} {1 2 3 4 5 6 7 8 9 10}
|
||||
|
||||
do_test 2.1.3 {
|
||||
sqlite3_snapshot_free $snapshot
|
||||
execsql COMMIT
|
||||
execsql COMMIT db2
|
||||
db2 close
|
||||
} {}
|
||||
|
||||
#-------------------------------------------------------------------------
|
||||
# Check some errors in sqlite3_snapshot_open(). It is an error if:
|
||||
#
|
||||
|
||||
Loading…
Reference in New Issue
Block a user