Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor client cancel requests in C /C++ code #232

Closed
woodbri opened this issue Jan 6, 2014 · 7 comments · Fixed by #1369
Closed

Honor client cancel requests in C /C++ code #232

woodbri opened this issue Jan 6, 2014 · 7 comments · Fixed by #1369

Comments

@woodbri
Copy link
Contributor

woodbri commented Jan 6, 2014

Currently our C/C++ code does not honor client cancel query requests. There is a macro CHECK_FOR_INTERRUPTS() defined in server/miscadmin.h and we should use that or more likely mimic it so we can check for a pending interrupt in any long running processes, then do clean up and finally call ProcessInterrupts().

We will need to look at how to do this from C++ it might be as simple as wrapping the include like:

extern "C" {
#include "server/miscadmin.h"
}
@cvvergara
Copy link
Member

Fixed this issue for pgr_ksp with commit 5cdd106.
TODO:
Make the code available to all functions
And each function, if required, should have to add where appropriate the catch

@dkastl dkastl added this to the Release 2.2.0 milestone Mar 23, 2015
@dkastl dkastl removed 2.0 labels Mar 23, 2015
@cvvergara cvvergara modified the milestones: Release 3.0.0, Release 2.2.0 Dec 3, 2015
@woodbri
Copy link
Contributor Author

woodbri commented Feb 25, 2016

In the cpp code where it is in a loop that takes a a lot of iterations you need to include THROW_ON_SIGINT

while (lots_of_stuff_to_do) {
  ...
  THROW_ON_SIGINT
}

and #include "signalhandler.h"

This is a very low cost asynchronous way to check if we got a ^C and the wrapper needs to catch this and inform the C code. You can freely put THROW_ON_SIGINT through out the code and you will get a faster terminate to the ^C

@woodbri
Copy link
Contributor Author

woodbri commented Sep 1, 2016

I'm not sure, but maybe the compiler is not scoping the #define in
https://github.com/cvvergara/pgrouting/blob/fix/issue232/src/common/src/signalhandler.h#L148

sigint_handler is a reference to
https://github.com/cvvergara/pgrouting/blob/fix/issue232/src/common/src/signalhandler.h#L142

you could try rewriting L148 to be something like:

#define THROW_ON_SIGINT do { \
    if (SignalHandler::instance()->registerHandler(SIGINT, &sigint_handler).gracefulQuit() == 1 ) \
        throw(UserQuitException("Abort on User Request!")); \
} while (0);

and see if that helps. Have the rules for scoping #define objects changed since C++98?

@cvvergara
Copy link
Member

I dont think scoping rules changed.
I missed that. SignalHandler::instance()-> I'll try again with it. (forgot its kind of a singleton)

@cvvergara
Copy link
Member

could not make it work :(

@cvvergara cvvergara removed this from the Release 3.0.0 milestone Jul 24, 2019
@woodbri
Copy link
Contributor Author

woodbri commented Jul 24, 2019

Thank you for following up on this. Its been too long for me to remember the details. Removing from 3.0 is fine.

I think to diagnosis this issue in the future we need to trace where if anywhere the signal is getting handled. So for example:

  1. is the signal handler getting installed correctly?
  2. is the backend process getting the cancel request?
  3. is our code checking for the the cancel request?
  4. is our code handling the request?
  5. etc

Once this is understood for one command, it should be easy to propagate, so maybe making a dummy command just for testing, that loop 30 times with a sleep(1sec) in the loop would be adequate for testing.

mbakli added a commit to mbakli/pgrouting that referenced this issue Jun 29, 2020
Check interruption in include/dijkstra

Check interruption in include/allpairs

Check interruption in include/astar

Check interruption in include/dijkstra

Check interruption in include/bellman_ford

Check interruption in include/breadthFirstSearch

Check interruption in include/contraction

Check interruption in include/dagShortestPath

Check interruption in include/max_flow

Check interruption in include/spanningTree

Check interruption in include/topologicalSort

Check interruption in src/alpha_shape

Check interruption in src/components

Suppress the -Wconversion warning in src/trsp/trsp.c

License

ifndef error

include

Fix cmake

Release notes

NEWS file

Honoring cancellation request (closes pgRouting#232)
* allpairs
* astar
* bellman_ford
* breadthFirstSearch
* dagShortestPath
* dijsktra
* contraction
* cpp_common
* max_flow
* spanningTree
* topologicalSort
* alpha_shape
* components
* trsp
mbakli added a commit to mbakli/pgrouting that referenced this issue Jul 3, 2020
* allpairs
* astar
* bellman_ford
* breadthFirstSearch
* dagShortestPath
* dijsktra
* contraction
* cpp_common
* max_flow
* spanningTree
* topologicalSort
* alpha_shape
* components
* trsp
mbakli added a commit to mbakli/pgrouting that referenced this issue Jul 3, 2020
* allpairs
* astar
* bellman_ford
* breadthFirstSearch
* dagShortestPath
* dijsktra
* contraction
* cpp_common
* max_flow
* spanningTree
* topologicalSort
* alpha_shape
* components
* trsp
@cvvergara cvvergara added this to the Release 3.0.1 milestone Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants