search

plasmodic / ecto

ecto is a dynamically configurable Directed Acyclic processing Graph (DAG) framework.
http://ecto.willowgarage.com/
BSD 3-Clause "New" or "Revised" License
98 stars 37 forks source link

Posix signal handling #250

Closed stonier closed 10 years ago

stonier commented 10 years ago

Problem

Commit dd8fef0ec00cbb9e489aa9156206e4f0508ac12c introduce a big change in the scheduler - multithreading was dropped and async support modified. In the process, signal handling got disconnected (note the #if 0).

This means that the python ectoscript controlling this c++ execution would only receive a notification of the signal via PyErr_SetInterrupt() only after any c++ execution terminated.

In the case of an execute() call, or execute(niter=_some_large_number). This can be a very long time.

Solutions

You can force the python ecto script to check for signals with PyErr_CheckSignals(), see Ethan's comment but this may be a bad way of doing it. Perhaps it clobbers the underlying c++ execution? Better would be to get the c++ execution to terminate gracefully.

All I've done here is set a flag that replicates the ecto::QUIT behaviour. Another way would be to trigger it via stop() but I'm unclear on who or what calls that yet. It has issues trying that with the current code as well.

Thoughts?

stonier commented 10 years ago

PyErr_CheckSignals() definitely not an option. You want the underlying execution to finish gracefully.

stonier commented 10 years ago

I've moved the signal handling to cover sync and async executors. Will do some more testing before giving the green light on this one.

stonier commented 10 years ago

:white_check_mark: been working well for us in a variety of situations.

vrabaud commented 10 years ago

thx on that one ! all tests are still fine.