Skip to content

Commit fe31b35

Browse files
penbergpenberg
authored
libsql-sqlite3: Make libsql_stmt_interrupt() abort an in-flight step (#2248)
libsql_stmt_interrupt() set a per-statement isInterrupted flag, but the VDBE execution loop only ever checked the connection-wide db->u1.isInterrupted. A statement already executing inside sqlite3_step() therefore ran to completion regardless of the request; the flag was only observed at the next step() entry. Check p->isInterrupted alongside db->u1.isInterrupted at both VDBE interrupt-check sites so an interrupt requested mid-execution aborts the running statement promptly with SQLITE_INTERRUPT, without touching the connection-wide interrupt state (other statements keep running). Set and clear the flag atomically, mirroring sqlite3_interrupt(), since the request may come from another thread. Tests: - test/interruptstmt.test: deterministic regression via a new sqlite_stmt_interrupt_count test hook, covering the in-loop check and the connection-flag-stays-clear property. - test/interrupttest.c: standalone multi-threaded test of the real cross-thread case (interrupt a step() in flight) and statement-level granularity. Build and run with `make interrupttest`.
2 parents 5cdc619 + f29dd3a commit fe31b35

11 files changed

Lines changed: 393 additions & 25 deletions

File tree

‎Cargo.lock‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎libsql-ffi/bundled/SQLite3MultipleCiphers/src/sqlite3.c‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
** src/sqlite3ext.h
7171
** src/sqliteInt.h
7272
** src/status.c
73+
** src/test1.c
7374
** src/test2.c
7475
** src/test3.c
7576
** src/test8.c
@@ -90201,7 +90202,7 @@ SQLITE_PRIVATE int sqlite3VdbeReset(Vdbe *p){
9020190202
#ifdef SQLITE_DEBUG
9020290203
p->nWrite = 0;
9020390204
#endif
90204-
p->isInterrupted = 0;
90205+
AtomicStore(&p->isInterrupted, 0);
9020590206

9020690207
/* Save profiling information from this VDBE run.
9020790208
*/
@@ -93043,7 +93044,9 @@ void libsql_stmt_interrupt(sqlite3_stmt *pStmt){
9304393044
(void)SQLITE_MISUSE_BKPT;
9304493045
return;
9304593046
}
93046-
v->isInterrupted = 1;
93047+
/* Set atomically: this may be called from a different thread than the one
93048+
** executing the statement, mirroring sqlite3_interrupt(). */
93049+
AtomicStore(&v->isInterrupted, 1);
9304793050
}
9304893051

9304993052
/*
@@ -93061,7 +93064,7 @@ SQLITE_API int sqlite3_step(sqlite3_stmt *pStmt){
9306193064
return SQLITE_MISUSE_BKPT;
9306293065
}
9306393066
db = v->db;
93064-
if( v->isInterrupted ){
93067+
if( AtomicLoad(&v->isInterrupted) ){
9306593068
rc = SQLITE_INTERRUPT;
9306693069
v->rc = rc;
9306793070
db->errCode = rc;
@@ -95105,6 +95108,12 @@ SQLITE_API int sqlite3_search_count = 0;
9510595108
*/
9510695109
#ifdef SQLITE_TEST
9510795110
SQLITE_API int sqlite3_interrupt_count = 0;
95111+
/*
95112+
** As above, but simulates a statement-level interrupt
95113+
** (libsql_stmt_interrupt()) on the statement that is currently executing,
95114+
** rather than a connection-wide sqlite3_interrupt().
95115+
*/
95116+
SQLITE_API int sqlite3_stmt_interrupt_count = 0;
9510895117
#endif
9510995118

9511095119
/*
@@ -95928,7 +95937,9 @@ SQLITE_PRIVATE int sqlite3VdbeExec(
9592895937
p->iCurrentTime = 0;
9592995938
assert( p->explain==0 );
9593095939
db->busyHandler.nBusy = 0;
95931-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
95940+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
95941+
goto abort_due_to_interrupt;
95942+
}
9593295943
sqlite3VdbeIOTraceSql(p);
9593395944
#ifdef SQLITE_DEBUG
9593495945
sqlite3BeginBenignMalloc();
@@ -95997,6 +96008,12 @@ SQLITE_PRIVATE int sqlite3VdbeExec(
9599796008
sqlite3_interrupt(db);
9599896009
}
9599996010
}
96011+
if( sqlite3_stmt_interrupt_count>0 ){
96012+
sqlite3_stmt_interrupt_count--;
96013+
if( sqlite3_stmt_interrupt_count==0 ){
96014+
libsql_stmt_interrupt((sqlite3_stmt*)p);
96015+
}
96016+
}
9600096017
#endif
9600196018

9600296019
/* Sanity checking on other operands */
@@ -96118,7 +96135,9 @@ case OP_Goto: { /* jump */
9611896135
** checks on every opcode. This helps sqlite3_step() to run about 1.5%
9611996136
** faster according to "valgrind --tool=cachegrind" */
9612096137
check_for_interrupt:
96121-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
96138+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
96139+
goto abort_due_to_interrupt;
96140+
}
9612296141
#ifndef SQLITE_OMIT_PROGRESS_CALLBACK
9612396142
/* Call the progress callback if it is configured and the required number
9612496143
** of VDBE ops have been executed (either since this invocation of
@@ -104463,7 +104482,7 @@ default: { /* This is really OP_Noop, OP_Explain */
104463104482
** flag.
104464104483
*/
104465104484
abort_due_to_interrupt:
104466-
assert( AtomicLoad(&db->u1.isInterrupted) );
104485+
assert( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) );
104467104486
rc = SQLITE_INTERRUPT;
104468104487
goto abort_due_to_error;
104469104488
}

‎libsql-ffi/bundled/src/sqlite3.c‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
** src/sqlite3ext.h
7171
** src/sqliteInt.h
7272
** src/status.c
73+
** src/test1.c
7374
** src/test2.c
7475
** src/test3.c
7576
** src/test8.c
@@ -90201,7 +90202,7 @@ SQLITE_PRIVATE int sqlite3VdbeReset(Vdbe *p){
9020190202
#ifdef SQLITE_DEBUG
9020290203
p->nWrite = 0;
9020390204
#endif
90204-
p->isInterrupted = 0;
90205+
AtomicStore(&p->isInterrupted, 0);
9020590206

9020690207
/* Save profiling information from this VDBE run.
9020790208
*/
@@ -93043,7 +93044,9 @@ void libsql_stmt_interrupt(sqlite3_stmt *pStmt){
9304393044
(void)SQLITE_MISUSE_BKPT;
9304493045
return;
9304593046
}
93046-
v->isInterrupted = 1;
93047+
/* Set atomically: this may be called from a different thread than the one
93048+
** executing the statement, mirroring sqlite3_interrupt(). */
93049+
AtomicStore(&v->isInterrupted, 1);
9304793050
}
9304893051

9304993052
/*
@@ -93061,7 +93064,7 @@ SQLITE_API int sqlite3_step(sqlite3_stmt *pStmt){
9306193064
return SQLITE_MISUSE_BKPT;
9306293065
}
9306393066
db = v->db;
93064-
if( v->isInterrupted ){
93067+
if( AtomicLoad(&v->isInterrupted) ){
9306593068
rc = SQLITE_INTERRUPT;
9306693069
v->rc = rc;
9306793070
db->errCode = rc;
@@ -95105,6 +95108,12 @@ SQLITE_API int sqlite3_search_count = 0;
9510595108
*/
9510695109
#ifdef SQLITE_TEST
9510795110
SQLITE_API int sqlite3_interrupt_count = 0;
95111+
/*
95112+
** As above, but simulates a statement-level interrupt
95113+
** (libsql_stmt_interrupt()) on the statement that is currently executing,
95114+
** rather than a connection-wide sqlite3_interrupt().
95115+
*/
95116+
SQLITE_API int sqlite3_stmt_interrupt_count = 0;
9510895117
#endif
9510995118

9511095119
/*
@@ -95928,7 +95937,9 @@ SQLITE_PRIVATE int sqlite3VdbeExec(
9592895937
p->iCurrentTime = 0;
9592995938
assert( p->explain==0 );
9593095939
db->busyHandler.nBusy = 0;
95931-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
95940+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
95941+
goto abort_due_to_interrupt;
95942+
}
9593295943
sqlite3VdbeIOTraceSql(p);
9593395944
#ifdef SQLITE_DEBUG
9593495945
sqlite3BeginBenignMalloc();
@@ -95997,6 +96008,12 @@ SQLITE_PRIVATE int sqlite3VdbeExec(
9599796008
sqlite3_interrupt(db);
9599896009
}
9599996010
}
96011+
if( sqlite3_stmt_interrupt_count>0 ){
96012+
sqlite3_stmt_interrupt_count--;
96013+
if( sqlite3_stmt_interrupt_count==0 ){
96014+
libsql_stmt_interrupt((sqlite3_stmt*)p);
96015+
}
96016+
}
9600096017
#endif
9600196018

9600296019
/* Sanity checking on other operands */
@@ -96118,7 +96135,9 @@ case OP_Goto: { /* jump */
9611896135
** checks on every opcode. This helps sqlite3_step() to run about 1.5%
9611996136
** faster according to "valgrind --tool=cachegrind" */
9612096137
check_for_interrupt:
96121-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
96138+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
96139+
goto abort_due_to_interrupt;
96140+
}
9612296141
#ifndef SQLITE_OMIT_PROGRESS_CALLBACK
9612396142
/* Call the progress callback if it is configured and the required number
9612496143
** of VDBE ops have been executed (either since this invocation of
@@ -104463,7 +104482,7 @@ default: { /* This is really OP_Noop, OP_Explain */
104463104482
** flag.
104464104483
*/
104465104484
abort_due_to_interrupt:
104466-
assert( AtomicLoad(&db->u1.isInterrupted) );
104485+
assert( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) );
104467104486
rc = SQLITE_INTERRUPT;
104468104487
goto abort_due_to_error;
104469104488
}

‎libsql-sqlite3/Makefile.in‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,11 @@ threadtest: threadtest3$(TEXE)
16821682
threadtest5: sqlite3.c $(TOP)/test/threadtest5.c
16831683
$(LTLINK) $(TOP)/test/threadtest5.c sqlite3.c -o $@ $(TLIBS)
16841684

1685+
# Multi-threaded test of the libsql_stmt_interrupt() API. Builds a small
1686+
# standalone binary; run it directly to check the result.
1687+
interrupttest: sqlite3.c $(TOP)/test/interrupttest.c
1688+
$(LTLINK) $(TOP)/test/interrupttest.c sqlite3.c -o $@ $(TLIBS)
1689+
16851690
# Standard install and cleanup targets
16861691
#
16871692
liblibsql_wasm_install: liblibsql_wasm

‎libsql-sqlite3/main.mk‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,11 @@ threadtest3$(EXE): sqlite3.o $(THREADTEST3_SRC) $(TOP)/src/test_multiplex.c
10471047
threadtest: threadtest3$(EXE)
10481048
./threadtest3$(EXE)
10491049

1050+
# Multi-threaded test of the libsql_stmt_interrupt() API. Builds a small
1051+
# standalone binary; run it directly to check the result.
1052+
interrupttest$(EXE): sqlite3.o $(TOP)/test/interrupttest.c
1053+
$(TCCX) $(TOP)/test/interrupttest.c sqlite3.o -o $@ $(THREADLIB)
1054+
10501055
TEST_EXTENSION = $(SHPREFIX)testloadext.$(SO)
10511056
$(TEST_EXTENSION): $(TOP)/src/test_loadext.c
10521057
$(MKSHLIB) $(TOP)/src/test_loadext.c -o $(TEST_EXTENSION)

‎libsql-sqlite3/src/test1.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8970,6 +8970,7 @@ int Sqlitetest1_Init(Tcl_Interp *interp){
89708970
extern int sqlite3_search_count;
89718971
extern int sqlite3_found_count;
89728972
extern int sqlite3_interrupt_count;
8973+
extern int sqlite3_stmt_interrupt_count;
89738974
extern int sqlite3_open_file_count;
89748975
extern int sqlite3_sort_count;
89758976
extern int sqlite3_current_time;
@@ -9306,8 +9307,10 @@ int Sqlitetest1_Init(Tcl_Interp *interp){
93069307
(char*)&sqlite3_max_blobsize, TCL_LINK_INT);
93079308
Tcl_LinkVar(interp, "sqlite_like_count",
93089309
(char*)&sqlite3_like_count, TCL_LINK_INT);
9309-
Tcl_LinkVar(interp, "sqlite_interrupt_count",
9310+
Tcl_LinkVar(interp, "sqlite_interrupt_count",
93109311
(char*)&sqlite3_interrupt_count, TCL_LINK_INT);
9312+
Tcl_LinkVar(interp, "sqlite_stmt_interrupt_count",
9313+
(char*)&sqlite3_stmt_interrupt_count, TCL_LINK_INT);
93119314
Tcl_LinkVar(interp, "sqlite_open_file_count",
93129315
(char*)&sqlite3_open_file_count, TCL_LINK_INT);
93139316
Tcl_LinkVar(interp, "sqlite_current_time",

‎libsql-sqlite3/src/vdbe.c‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ int sqlite3_search_count = 0;
7272
*/
7373
#ifdef SQLITE_TEST
7474
int sqlite3_interrupt_count = 0;
75+
/*
76+
** As above, but simulates a statement-level interrupt
77+
** (libsql_stmt_interrupt()) on the statement that is currently executing,
78+
** rather than a connection-wide sqlite3_interrupt().
79+
*/
80+
int sqlite3_stmt_interrupt_count = 0;
7581
#endif
7682

7783
/*
@@ -895,7 +901,9 @@ int sqlite3VdbeExec(
895901
p->iCurrentTime = 0;
896902
assert( p->explain==0 );
897903
db->busyHandler.nBusy = 0;
898-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
904+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
905+
goto abort_due_to_interrupt;
906+
}
899907
sqlite3VdbeIOTraceSql(p);
900908
#ifdef SQLITE_DEBUG
901909
sqlite3BeginBenignMalloc();
@@ -964,6 +972,12 @@ int sqlite3VdbeExec(
964972
sqlite3_interrupt(db);
965973
}
966974
}
975+
if( sqlite3_stmt_interrupt_count>0 ){
976+
sqlite3_stmt_interrupt_count--;
977+
if( sqlite3_stmt_interrupt_count==0 ){
978+
libsql_stmt_interrupt((sqlite3_stmt*)p);
979+
}
980+
}
967981
#endif
968982

969983
/* Sanity checking on other operands */
@@ -1085,7 +1099,9 @@ case OP_Goto: { /* jump */
10851099
** checks on every opcode. This helps sqlite3_step() to run about 1.5%
10861100
** faster according to "valgrind --tool=cachegrind" */
10871101
check_for_interrupt:
1088-
if( AtomicLoad(&db->u1.isInterrupted) ) goto abort_due_to_interrupt;
1102+
if( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) ){
1103+
goto abort_due_to_interrupt;
1104+
}
10891105
#ifndef SQLITE_OMIT_PROGRESS_CALLBACK
10901106
/* Call the progress callback if it is configured and the required number
10911107
** of VDBE ops have been executed (either since this invocation of
@@ -9430,7 +9446,7 @@ default: { /* This is really OP_Noop, OP_Explain */
94309446
** flag.
94319447
*/
94329448
abort_due_to_interrupt:
9433-
assert( AtomicLoad(&db->u1.isInterrupted) );
9449+
assert( AtomicLoad(&db->u1.isInterrupted) || AtomicLoad(&p->isInterrupted) );
94349450
rc = SQLITE_INTERRUPT;
94359451
goto abort_due_to_error;
94369452
}

‎libsql-sqlite3/src/vdbeapi.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,9 @@ void libsql_stmt_interrupt(sqlite3_stmt *pStmt){
897897
(void)SQLITE_MISUSE_BKPT;
898898
return;
899899
}
900-
v->isInterrupted = 1;
900+
/* Set atomically: this may be called from a different thread than the one
901+
** executing the statement, mirroring sqlite3_interrupt(). */
902+
AtomicStore(&v->isInterrupted, 1);
901903
}
902904

903905
/*
@@ -915,7 +917,7 @@ int sqlite3_step(sqlite3_stmt *pStmt){
915917
return SQLITE_MISUSE_BKPT;
916918
}
917919
db = v->db;
918-
if( v->isInterrupted ){
920+
if( AtomicLoad(&v->isInterrupted) ){
919921
rc = SQLITE_INTERRUPT;
920922
v->rc = rc;
921923
db->errCode = rc;

‎libsql-sqlite3/src/vdbeaux.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3632,7 +3632,7 @@ int sqlite3VdbeReset(Vdbe *p){
36323632
#ifdef SQLITE_DEBUG
36333633
p->nWrite = 0;
36343634
#endif
3635-
p->isInterrupted = 0;
3635+
AtomicStore(&p->isInterrupted, 0);
36363636

36373637
/* Save profiling information from this VDBE run.
36383638
*/

0 commit comments

Comments
 (0)