Implement python cleanup handlers

5 files changed, 87 insertions, 41 deletions

diff --git a/backends/lua.c b/backends/lua.c
index 7424f65..127933a 100644
--- a/backends/lua.c
+++ b/backends/lua.c
@@ -274,10 +274,13 @@ static int lua_callback_cleanup_handler(lua_State* interpreter){
 		return 0;
 	}
 
-	luaL_checktype(interpreter, 1, LUA_TFUNCTION);
+	if(lua_type(interpreter, 1) != LUA_TFUNCTION && lua_type(interpreter, 1) != LUA_TNIL){
+		LOG("Cleanup handler function parameter was neither nil nor a function");
+		return 0;
+	}
 
 	data->cleanup_handler = luaL_ref(interpreter, LUA_REGISTRYINDEX);
-	if(current_handler == LUA_NOREF){
+	if(current_handler == LUA_NOREF || current_handler == LUA_REFNIL){
 		lua_pushnil(interpreter);
 		return 1;
 	}
@@ -656,7 +659,7 @@ static int lua_shutdown(size_t n, instance** inst){
 		data = (lua_instance_data*) inst[u]->impl;
 
 		//call cleanup function if one is registered
-		if(data->cleanup_handler != LUA_NOREF){
+		if(data->cleanup_handler != LUA_NOREF && data->cleanup_handler != LUA_REFNIL){
 			lua_rawgeti(data->interpreter, LUA_REGISTRYINDEX, data->cleanup_handler);
 			lua_pcall(data->interpreter, 0, 0, 0);
 		}
diff --git a/backends/lua.md b/backends/lua.md
index 30d7580..e59e513 100644
--- a/backends/lua.md
+++ b/backends/lua.md
@@ -15,13 +15,13 @@ The following functions are provided within the Lua interpreter for interaction
 |-------------------------------|-------------------------------|---------------------------------------|
 | `output(string, number)`	| `output("foo", 0.75)`		| Output a value event to a channel on this instance	|
 | `interval(function, number)`	| `interval(update, 100)`	| Register a function to be called periodically. Intervals are milliseconds (rounded to the nearest 10 ms). Calling `interval` on a Lua function multiple times updates the interval. Specifying `0` as interval stops periodic calls to the function |
+| `cleanup_handler(function)`	| `cleanup_handler(shutdown)`	| Register a function to be called when the instance is destroyed (on MIDIMonster shutdown). One cleanup handler can be registered per instance. Calling this function when the instance already has a cleanup handler registered replaces the handler, returning the old one. |
 | `input_value(string)`		| `input_value("foo")`		| Get the last input value on a channel on this instance	|
 | `output_value(string)`	| `output_value("bar")`		| Get the last output value on a channel on this instance |
 | `input_channel()`		| `print(input_channel())`	| Returns the name of the input channel whose handler function is currently running or `nil` if in an `interval`'ed function (or the initial parse step) |
 | `timestamp()`			| `print(timestamp())`		| Returns the core timestamp for this iteration with millisecond resolution. This is not a performance timer, but intended for timeouting, etc |
 | `thread(function)`		| `thread(run_show)`		| Run a function as a Lua thread (see below) |
 | `sleep(number)`		| `sleep(100)`			| Suspend current thread for time specified in milliseconds |
-| `cleanup_handler(function)`	|				| Register a function to be called when the instance is destroyed (on MIDIMonster shutdown). One cleanup handler can be registered per instance. Calling this function when the instance already has a cleanup handler registered replaces the handler, returning the old one. |
 
 Example script:
 ```lua
@@ -45,6 +45,7 @@ function run_show()
 end
 
 function save_values()
+	-- Store state to a file, for example
 end
 
 interval(toggle, 1000)
diff --git a/backends/python.c b/backends/python.c
index 9f1d642..4c9248d 100644
--- a/backends/python.c
+++ b/backends/python.c
@@ -257,6 +257,33 @@ static PyObject* mmpy_interval(PyObject* self, PyObject* args){
 	return Py_None;
 }
 
+static PyObject* mmpy_cleanup_handler(PyObject* self, PyObject* args){
+	instance* inst = *((instance**) PyModule_GetState(self));
+	python_instance_data* data = (python_instance_data*) inst->impl;
+	PyObject* current_handler = data->cleanup_handler;
+
+	if(!PyArg_ParseTuple(args, "O", &(data->cleanup_handler))
+			|| (data->cleanup_handler != Py_None && !PyCallable_Check(data->cleanup_handler))){
+		data->cleanup_handler = current_handler;
+		return NULL;
+	}
+
+	if(data->cleanup_handler == Py_None){
+		data->cleanup_handler = NULL;
+	}
+	else{
+		Py_INCREF(data->cleanup_handler);
+	}
+
+	if(!current_handler){
+		Py_INCREF(Py_None);
+		return Py_None;
+	}
+
+	Py_DECREF(current_handler);
+	return current_handler;
+}
+
 static PyObject* mmpy_manage_fd(PyObject* self, PyObject* args){
 	instance* inst = *((instance**) PyModule_GetState(self));
 	python_instance_data* data = (python_instance_data*) inst->impl;
@@ -264,12 +291,10 @@ static PyObject* mmpy_manage_fd(PyObject* self, PyObject* args){
 	size_t u = 0, last_free = 0;
 	int fd = -1;
 
-	if(!PyArg_ParseTuple(args, "OO", &handler, &sock)){
-		return NULL;
-	}
-
-	if(handler != Py_None && !PyCallable_Check(handler)){
-		PyErr_SetString(PyExc_TypeError, "manage() requires either None or a callable");
+	if(!PyArg_ParseTuple(args, "OO", &handler, &sock)
+			|| sock == Py_None
+			|| (handler != Py_None && !PyCallable_Check(handler))){
+		PyErr_SetString(PyExc_TypeError, "manage() requires either None or a callable and a socket-like object");
 		return NULL;
 	}
 
@@ -398,13 +423,14 @@ static PyObject* mmpy_init(){
 	};
 
 	static PyMethodDef mmpy_methods[] = {
-		{"output", mmpy_output, METH_VARARGS, "Output a channel event"},
-		{"inputvalue", mmpy_input_value, METH_VARARGS, "Get last input value for a channel"},
-		{"outputvalue", mmpy_output_value, METH_VARARGS, "Get the last output value for a channel"},
+		{"output", mmpy_output, METH_VARARGS, "Output a channel event on the instance"},
+		{"inputvalue", mmpy_input_value, METH_VARARGS, "Get last input value for a channel on the instance"},
+		{"outputvalue", mmpy_output_value, METH_VARARGS, "Get the last output value for a channel on the instance"},
 		{"current", mmpy_current_handler, METH_VARARGS, "Get the name of the currently executing channel handler"},
 		{"timestamp", mmpy_timestamp, METH_VARARGS, "Get the core timestamp (in milliseconds)"},
 		{"manage", mmpy_manage_fd, METH_VARARGS, "(Un-)register a socket or file descriptor for notifications"},
 		{"interval", mmpy_interval, METH_VARARGS, "Register or update an interval handler"},
+		{"cleanup_handler", mmpy_cleanup_handler, METH_VARARGS, "Register or update the instances cleanup handler"},
 		{0}
 	};
 
@@ -674,29 +700,44 @@ static int python_start(size_t n, instance** inst){
 
 static int python_shutdown(size_t n, instance** inst){
 	size_t u, p;
+	PyObject* result = NULL;
 	python_instance_data* data = NULL;
 
-	//clean up channels
-	//this needs to be done before stopping the interpreters,
-	//because the handler references are refcounted
-	for(u = 0; u < n; u++){
-		data = (python_instance_data*) inst[u]->impl;
-		for(p = 0; p < data->channels; p++){
-			free(data->channel[p].name);
-			Py_XDECREF(data->channel[p].handler);
+	//if there are no instances, the python interpreter is not started, so cleanup can be skipped
+	if(python_main){
+		//release interval references
+		for(p = 0; p < intervals; p++){
+			//swap to interpreter
+			PyEval_RestoreThread(interval[p].interpreter);
+			Py_XDECREF(interval[p].reference);
+			PyEval_ReleaseThread(interval[p].interpreter);
 		}
-		free(data->channel);
-		free(data->default_handler);
-		//do not free data here, needed for shutting down interpreters
-	}
 
-	if(python_main){
-		//just used to lock the GIL
+		//lock the GIL for later interpreter release
 		PyEval_RestoreThread(python_main);
 
 		for(u = 0; u < n; u++){
 			data = (python_instance_data*) inst[u]->impl;
 
+			//swap to interpreter to be safe for releasing the references
+			PyThreadState_Swap(data->interpreter);
+
+			//run cleanup handler before cleaning up channel data to allow reading channel data
+			if(data->cleanup_handler){
+				result = PyObject_CallFunction(data->cleanup_handler, NULL);
+				Py_XDECREF(result);
+				Py_XDECREF(data->cleanup_handler);
+			}
+
+			//clean up channels
+			for(p = 0; p < data->channels; p++){
+				free(data->channel[p].name);
+				Py_XDECREF(data->channel[p].handler);
+			}
+			free(data->channel);
+			free(data->default_handler);
+			Py_XDECREF(data->handler);
+
 			//close sockets
 			for(p = 0; p < data->sockets; p++){
 				close(data->socket[p].fd); //FIXME does python do this on its own?
@@ -704,26 +745,18 @@ static int python_shutdown(size_t n, instance** inst){
 				Py_XDECREF(data->socket[p].handler);
 			}
 
-			//release interval references
-			for(p = 0; p <intervals; p++){
-				Py_XDECREF(interval[p].reference);
-			}
-			Py_XDECREF(data->handler);
-
+			//shut down interpreter, GIL is held after this but state is NULL
 			DBGPF("Shutting down interpreter for instance %s", inst[u]->name);
-			//swap to interpreter and end it, GIL is held after this but state is NULL
-			PyThreadState_Swap(data->interpreter);
 			PyErr_Clear();
 			//PyThreadState_Clear(data->interpreter);
 			Py_EndInterpreter(data->interpreter);
-
 			free(data);
 		}
 
 		//shut down main interpreter
 		PyThreadState_Swap(python_main);
 		if(Py_FinalizeEx()){
-			LOG("Failed to destroy python interpreters");
+			LOG("Failed to shut down python library");
 		}
 		PyMem_RawFree(program_name);
 	}
diff --git a/backends/python.h b/backends/python.h
index 020aeac..539389b 100644
--- a/backends/python.h
+++ b/backends/python.h
@@ -45,4 +45,5 @@ typedef struct /*_python_instance_data*/ {
 
 	char* default_handler;
 	PyObject* handler;
+	PyObject* cleanup_handler;
 } python_instance_data;
diff --git a/backends/python.md b/backends/python.md
index 6852a79..ab0fb38 100644
--- a/backends/python.md
+++ b/backends/python.md
@@ -17,13 +17,14 @@ The `midimonster` module provides the following functions:
 
 | Function			| Usage example				| Description					|
 |-------------------------------|---------------------------------------|-----------------------------------------------|
-| `output(string, float)`	| `midimonster.output("foo", 0.75)`	| Output a value event to a channel		|
-| `inputvalue(string)`		| `midimonster.inputvalue("foo")`	| Get the last input value on a channel		|
-| `outputvalue(string)`		| `midimonster.outputvalue("bar")`	| Get the last output value on a channel 	|
+| `output(string, float)`	| `midimonster.output("foo", 0.75)`	| Output a value event to a channel on this instance |
+| `inputvalue(string)`		| `midimonster.inputvalue("foo")`	| Get the last input value on a channel	of this instance |
+| `outputvalue(string)`		| `midimonster.outputvalue("bar")`	| Get the last output value on a channel of this instance |
 | `current()`			| `print(midimonster.current())`	| Returns the name of the input channel whose handler function is currently running or `None` if the interpreter was called from another context |
 | `timestamp()`			| `print(midimonster.timestamp())`	| Get the internal core timestamp (in milliseconds)	|
 | `interval(function, long)`	| `midimonster.interval(toggle, 100)`	| Register a function to be called periodically. Interval is specified in milliseconds (accurate to 10msec). Calling `interval` with the same function again updates the interval. Specifying the interval as `0` cancels the interval |
-| `manage(function, socket)`	| `midimonster.manage(handler, socket)`| Register a (connected/listening) socket to the MIDIMonster core. Calls `function(socket)` when the socket is ready to read. Calling this method with `None` as the function argument unregisters the socket. A socket may only have one associated handler |
+| `manage(function, socket)`	| `midimonster.manage(handler, socket)`	| Register a (connected/listening) socket to the MIDIMonster core. Calls `function(socket)` when the socket is ready to read. Calling this method with `None` as the function argument unregisters the socket. A socket may only have one associated handler |
+| `cleanup_handler(function)`	| `midimonster.cleanup_handler(save_all)`| Register a function to be called when the instance is destroyed (on MIDIMonster shutdown). One cleanup handler can be registered per instance. Calling this function when the instance already has a cleanup handler registered replaces the handler, returning the old one. |
 
 Example Python module:
 ```python
@@ -48,12 +49,16 @@ def socket_handler(sock):
 def ping():
 	print(midimonster.timestamp())
 
+def save_positions():
+	# Store some data to disk
+
 # Register an interval
 midimonster.interval(ping, 1000)
 # Create and register a client socket (add error handling as you like)
 s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
 s.connect(("localhost", 8990))
 midimonster.manage(socket_handler, s)
+midimonster.cleanup_handler(save_positions)
 ```
 
 Input values range between 0.0 and 1.0, output values are clamped to the same range.
@@ -92,6 +97,9 @@ py1.out1 > py2.module.handler
 Output values will not trigger corresponding input event handlers unless the channel is mapped
 back in the MIDIMonster configuration. This is intentional.
 
+Output events generated from cleanup handlers called during shutdown will not be routed, as the core
+routing facility has already shut down at this point. There are no plans to change this behaviour.
+
 Importing a Python module named `midimonster` is probably a bad idea and thus unsupported.
 
 The MIDIMonster is, at its core, single-threaded. Do not try to use Python's `threading`