I'm getting a segfault when calling `sci_get_length(editor->sci)` in a function responding to an `editor-notify` signal. This occurs only when dropping files into Geany when no documents are open (the only document is "untitled"). This problem does not occur when:
* Double clicking a file in the file manager * Dragging a file when other documents are open * Draggin a file after modifying the untitled document
Checking that `editor`, `editor->document`, and `editor->sci` are not `NULL` does not prevent the segfault.
```C bool NewPluginClass::editor_notify(GObject *object, GeanyEditor *editor, SCNotification *nt, NewPluginClass *self) { if (!self->enable || !DOC_VALID(editor->document) || !editor->document->changed) { return false; }
if (editor && editor->sci && sci_get_length(editor->sci) == 0) { // testing }
return false; } ```
In the following, the last `printf` to output before the segfault is `3 SCNotification`. So apparently just testing `!editor->document->changed` is causing the segfault? ``` bool NewPluginClass::editor_notify(GObject *object, GeanyEditor *editor, SCNotification *nt, NewPluginClass *self) { if (!self->enable) { return false; }
printf("1 SCNotification = %lu\n", (ulong)nt); if (!editor) { printf("1a SCNotification = %lu\n", (ulong)nt); return false; }
printf("2 SCNotification = %lu\n", (ulong)nt); if (editor && !editor->document) { printf("2a SCNotification = %lu\n", (ulong)nt); return false; }
printf("3 SCNotification = %lu\n", (ulong)nt); if (editor && editor->document && !editor->document->changed) { printf("3a SCNotification = %lu\n", (ulong)nt); return false; }
printf("4 SCNotification = %lu\n", (ulong)nt); if (editor && !editor->sci) { printf("4a SCNotification = %lu\n", (ulong)nt); return false; }
printf("5 SCNotification = %lu\n", (ulong)nt); if (editor && editor->sci && sci_get_length(editor->sci) == 0) { printf("5a SCNotification = %lu\n", (ulong)nt); return false; } return false; } ```
The segfault does not occur with #2930.
Hmmm, ok, the edit you deleted, "Maybe a race condition where "untitled" is closed shortly before/after the editor-notify signal is sent?" might be close.
Did you connect to the editor notify signal "after"? In that case the standard Geany DND is actioned first, closing the untitled document, and then passing you its now defunct pointer.
"Maybe a race condition where "untitled" is closed shortly before/after the editor-notify signal is sent?"
That's in an edit I added to the first comment. The deleted comment was when I thought the problem was fixed by a PR, but it isn't. The plugin had stopped loading.
Did you connect to the editor notify signal "after"?
That does it. Changing "after" to false doesn't segfault. Changing back to true segfaults again.
Closed #2998.
"Maybe a race condition where "untitled" is closed shortly before/after the editor-notify signal is sent?"
But you were mostly right, true its not a "race" because its deterministic, but the idea was heading in the right direction.
its deterministic
So should this issue stay open because the validity checks pass?
No, its not fixable AFAICT (well not easily).
All signal callbacks connected to the same signal get passed the same parameters, so in this case the same pointer is passed to both. If a pointer in the parameters is invalidated by the first callback, the second will get a bad pointer, thats how it works.
Unfortunately removing the silly empty untitled is an intended function of the file open code.
Personally I'd be happy to see that untitled removed from the code, but some people are offended by the empty space if no files are open, that arguments been had, so its staying.
If the parameter to the callback was not a pointer to the object but was a pointer to the pointer to the object then it can be nulled if the object is invalidated, but that would break all Geany and plugins code that used the notify signal.
That it segfaults on drag-drop, but not double-click in a file manager makes it seem like just changing the order of some operations could fix it. Like... double click opens the doc first then kills untitled, but drag and drop kills untitled first then opens the dropped doc.
I don't think its different, the untitled is removed in `document_create()` called from `document_open_file_full()` which is used by both.
The difference is that the DND opens the file inside the signal handler ... hmmm, maybe the handler needs to return true if it invalidates any of the parameters.
But it doesn't look like the DND knows that the untitled is closed and the pointer invalidated.
github-comments@lists.geany.org