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.
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
