-
Notifications
You must be signed in to change notification settings - Fork 24
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #515 from duckdb/f-514-cancel
- Loading branch information
Showing
11 changed files
with
200 additions
and
21 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| #pragma once | ||
|
|
||
| #include <csignal> | ||
|
|
||
| #include "duckdb/common/shared_ptr.hpp" | ||
| #include "duckdb/main/client_context.hpp" | ||
|
|
||
| // Toy repo: https://github.com/krlmlr/cancel.test | ||
|
|
||
| namespace duckdb { | ||
|
|
||
| class ScopedInterruptHandler { | ||
| private: | ||
| shared_ptr<ClientContext> context; | ||
| bool interrupted = false; | ||
|
|
||
| // oldhandler stores the old signal handler | ||
| // so that it can be restored when the object is destroyed | ||
| typedef void (*sig_t)(int); | ||
| sig_t oldhandler; | ||
|
|
||
| static ScopedInterruptHandler *instance; | ||
|
|
||
| private: | ||
| ScopedInterruptHandler() = delete; | ||
| ScopedInterruptHandler(const ScopedInterruptHandler &) = delete; | ||
| ScopedInterruptHandler &operator=(const ScopedInterruptHandler &) = delete; | ||
| ScopedInterruptHandler(ScopedInterruptHandler &&) = delete; | ||
|
|
||
| public: | ||
| ScopedInterruptHandler(shared_ptr<ClientContext> context_); | ||
| ~ScopedInterruptHandler(); | ||
|
|
||
| bool HandleInterrupt() const; | ||
| void Disable(); | ||
|
|
||
| private: | ||
| static void signal_handler(int signum); | ||
| }; | ||
|
|
||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| #include "signal.hpp" | ||
|
|
||
| #include "cpp11/R.hpp" | ||
| #include "cpp11/function.hpp" | ||
| #include "cpp11/protect.hpp" // for safe | ||
| #include "duckdb/common/exception.hpp" | ||
|
|
||
| #include <R_ext/GraphicsEngine.h> | ||
|
|
||
| // Toy repo: https://github.com/krlmlr/cancel.test | ||
|
|
||
| namespace duckdb { | ||
|
|
||
| ScopedInterruptHandler *ScopedInterruptHandler::instance = nullptr; | ||
|
|
||
| ScopedInterruptHandler::ScopedInterruptHandler(shared_ptr<ClientContext> context_) : context(context_) { | ||
| if (instance) { | ||
| throw InternalException("ScopedInterruptHandler already active"); | ||
| } | ||
| if (context) { | ||
| instance = this; | ||
| oldhandler = std::signal(SIGINT, ScopedInterruptHandler::signal_handler); | ||
| } | ||
| } | ||
|
|
||
| ScopedInterruptHandler::~ScopedInterruptHandler() { | ||
| Disable(); | ||
| instance = nullptr; | ||
| } | ||
|
|
||
| bool ScopedInterruptHandler::HandleInterrupt() const { | ||
| // Never interrupted without context | ||
| if (!interrupted) { | ||
| return false; | ||
| } else { | ||
| D_ASSERT(context); | ||
| } | ||
|
|
||
| // This seems necessary to work around a specificity with the RStudio IDE on Windows. | ||
| // Without the message, the interrupt is not available as a catchable condition. | ||
| // https://github.com/krlmlr/cancel.test/issues/1 | ||
| cpp11::message(""); | ||
|
|
||
| // FIXME: Is this equivalent to cpp11::safe[Rf_onintrNoResume](), or worse? | ||
| cpp11::safe[Rf_onintr](); | ||
| return true; | ||
| } | ||
|
|
||
| void ScopedInterruptHandler::Disable() { | ||
| if (context) { | ||
| std::signal(SIGINT, oldhandler); | ||
| context.reset(); | ||
| } | ||
| } | ||
|
|
||
| void ScopedInterruptHandler::signal_handler(int signum) { | ||
| if (instance) { | ||
| instance->interrupted = true; | ||
| instance->context->Interrupt(); | ||
| } | ||
| } | ||
|
|
||
| }; // namespace duckdb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| test_that("long-running queries can be cancelled", { | ||
| skip_if_not_installed("callr") | ||
| # Skip on Windows for R < 4.4, the signal doesn't seem to make it through | ||
| # (but works for the toy repository) | ||
| skip_if(getRversion() < "4.4.0" && .Platform$OS.type == "windows") | ||
|
|
||
| r_session <- callr::r_session$new() | ||
|
|
||
| r_session$run(function() { | ||
| .GlobalEnv$con <- DBI::dbConnect(duckdb::duckdb()) | ||
| DBI::dbExecute(.GlobalEnv$con, "CREATE TABLE data AS SELECT unnest(generate_series(1, 100000)) AS a") | ||
| }) | ||
|
|
||
| r_session$call(function() { | ||
| .GlobalEnv$interrupted <- FALSE | ||
| tryCatch( | ||
| DBI::dbGetQuery(.GlobalEnv$con, "SELECT COUNT(*) FROM data JOIN data AS data2 ON data.a != data2.a"), | ||
| interrupt = function(e) { | ||
| .GlobalEnv$interrupted <- TRUE | ||
| } | ||
| ) | ||
| }) | ||
|
|
||
| start_time <- Sys.time() | ||
|
|
||
| Sys.sleep(0.2) | ||
| expect_equal(r_session$get_state(), "busy") | ||
| polled <- r_session$poll_process(200) | ||
| expect_equal(polled, "timeout") | ||
| r_session$interrupt() | ||
| polled <- r_session$poll_process(200) | ||
| expect_equal(polled, "ready") | ||
| expect_equal(r_session$read()$code, 200) | ||
| expect_equal(r_session$get_state(), "idle") | ||
|
|
||
| expect_true(r_session$run(function() .GlobalEnv$interrupted)) | ||
|
|
||
| end_time <- Sys.time() | ||
| expect_lt(end_time - start_time, 1) | ||
|
|
||
| r_session$run(function() DBI::dbDisconnect(.GlobalEnv$con)) | ||
| r_session$close() | ||
| }) |