Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Macros and Numbered Bookmarks plugins crash Geany #585

Closed
mocallins opened this issue Jun 21, 2017 · 12 comments · Fixed by #586
Closed

Macros and Numbered Bookmarks plugins crash Geany #585

mocallins opened this issue Jun 21, 2017 · 12 comments · Fixed by #586

Comments

@mocallins
Copy link

mocallins commented Jun 21, 2017

I've tried using on 2 different PC's Both Windows, 1 is 10 Enterprise and the other 8.1 Pro, both 64bit.

When I select the "Macros" option in the plugin manager is when it crashes.

@codebrainz
Copy link
Member

Confirmed on Windows 10 Pro x64 with Geany 1.30 and Windows 7 Professional SP1 x64 Geany 1.30.1.

@discorichardson
Copy link

I think Numbered Bookmarks crashes in the same way on windows. I think they both create a folder to store settings then crash Geany.

@codebrainz
Copy link
Member

Confirmed same issue with Numbered Bookmarks with same versions as above.

@discorichardson How do you know they both try and create folders before crashing? Any idea which folders? I hate debugging on Windows on release builds.

@codebrainz codebrainz changed the title Geanymacro plugin completely crashes Geany Macros and Numbered Bookmarks plugins crash Geany Jun 23, 2017
@codebrainz codebrainz added this to the 1.31.0 milestone Jun 23, 2017
@codebrainz
Copy link
Member

codebrainz commented Jun 23, 2017

I tested loading/unloading all of the other plugins and these are the only two which had this problem.

@eht16
Copy link
Member

eht16 commented Jun 23, 2017

I've got a stack trace which reveals it crashes somewhere in GDK, the last relevant frame in the stack is

#2  0x66b065ee in plugin_init ()
   from C:\Program Files (x86)\geany\lib\geany\geanymacro.dll

which doesn't help much.

I just triggered a full debug build on my machine to get a better stack trace.

@eht16
Copy link
Member

eht16 commented Jun 23, 2017

#2  0x66b065ee in plugin_init (data=0x6a3ae8e0 <geany_data>)
    at geanymacro.c:2219

which is

k=gdk_keymap_get_entries_for_keyval(NULL,'0'+i,&gdkkmkResults,&iResults);

I'll have a look at this at the weekend except anyone is faster.

@discorichardson
Copy link

discorichardson commented Jun 23, 2017

It looks like you've narrowed it down enough now.

In both cases after clicking checkbox to activate the plugin a folder is successfully created, and then Geany crashes.

The empty folders (on windows 7) are:
C:\Users\[username]\AppData\Roaming\geany\plugins\Geany_Numbered_Bookmarks
C:\Users\[username]\AppData\Roaming\geany\plugins\Geany_Macros

@codebrainz
Copy link
Member

@eht16 Is it the same function for Numbered Bookmarks?

@elextr
Copy link
Member

elextr commented Jun 23, 2017

Just FYI, the following message occurs on enabling these plugins on Linux, so I think @eht16 is in the right area.

(geany:9266): Gdk-CRITICAL **: gdk_keymap_get_entries_for_keyval: assertion 'GDK_IS_KEYMAP (keymap)' failed

@b4n
Copy link
Member

b4n commented Jun 23, 2017

https://developer.gnome.org/gdk2/stable/gdk2-Keyboard-Handling.html#gdk-keymap-get-entries-for-keyval

Note that passing NULL for keymap is deprecated and will stop to work in GTK+ 3.0. Use gdk_keymap_get_for_display() instead.

@eht16
Copy link
Member

eht16 commented Jun 24, 2017

@codebrainz yes, exactly the same issue with Numbered Bookmarks.

@b4n to the rescue. Thanks, that was exactly the error.

Interestingly, on Windows I don't see the warning Lex posted.
But GeanyLua is also affected.

Anyway, it's easy to fix. Will do a PR.

eht16 added a commit to eht16/geany-plugins that referenced this issue Jun 24, 2017
This fixes crashes and critial warnings.
Fixes geany#585.
@codebrainz
Copy link
Member

It sounds more like a GDK bug, it's weird something deprecated should cause a runtime assertion failure, and especially there's no reason it should crash the whole process. Oh well, GTK+ isn't exactly known for stability anymore :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants