Am 23.10.2013 16:53, schrieb Matthew Brush:
On 13-10-23 05:29 AM, n@sk0 wrote:
Before read : Keep in mind that i am *not* C/C++ "native" developer, and all message below can be just rant.
After little testing and debugging, i found that : In on_editor_notify() function, user_data is not a valid pointer : on_editor_notify(GObject *obj, gint scn, SCNotification *nt, gpointer user_data) { AutocloseUserData *data = user_data; ... }
After I playing for a while with this and reading demoplugin.c source, i found that in the autoclose.c, PluginCallback[] (line:829) is missing the definition for "editor-notify" event,so i added it, recompile plugin and now all seems to work correctly.
It's connected on line 812 inside a callback for when the document is activated. There's a few problems with this; the data allocated on L810 is going to leak, once for each time a document is activated (has tab switched to). The other thing is the plugin_signal_connect() is going to stack up signal handlers, so if you activated a document, on_editor_notify is going to get called on every single scintilla notification (keypress, cursor blink, etc.), for every number of times that document was activated.
The signal handler is connected for "document-new" and "document-open". This once per document and doesn't leak or stack anything.
The final problem is that, as Lex mentioned, it's not checking `DOC_VALID()` (or doc->is_valid) but just that data->doc != NULL, so if any document that was ever activated is closed, this is going to explode when the document pointer is dereferenced (for reasons I never understood, Geany "recycles" documents, so it's entirely possible to have a document pointer that is neither NULL nor valid).
True.
The crash in autoclose is a use-after-free one. The root problem is that the signal handler is not removed in the "document-close" callback, so it will be called again but this time the user_data points to freed memory. The fix (I'm going to make a PR) is to disconnect the handler in the "document-close" callback. Unforunately there is no plugin_signal_disconnect() (and plugin_signal_connect() eats the handler id) so one needs to hack around with g_signal_handlers_disconnect_by_func().
I introduced the buggy code, I'm sorry.
Best regards.