On 13-10-23 11:36 AM, Thomas Martitz wrote:
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.
Ah, my bad, I only looked for 5 minutes on way out to work, I assumed
on_document_activate() was used for "document-activate" signal.
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().
Yeah, I fought this before too :( We could probably add a new/better
function that returns the handler id and a matching function to
disconnect it by id, and just deprecate the old function. It seems like
it'd be quite easy.
Cheers,
Matthew Brush
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
