davatorium / rofi

Rofi: A window switcher, application launcher and dmenu replacement
https://davatorium.github.io/rofi/
Other
13.28k stars 612 forks source link

[BUG] Segfault when combi-modi and modi overlaps #1054

Closed georeth closed 4 years ago

georeth commented 4 years ago

Version

1.5.4-55-git-7a6fcb20-dirty (next)

Configuration

https://gist.github.com/georeth/da9184ffb85059551b6ac243dfb5c8c5

Launch Command

rofi -show combi -combi-modi run#drun#file-browser -modi combi#drun#file-browser -sidebar-mode -show-icons -theme /usr/share/rofi/themes/Pop-Dark.rasi -file-browser-dir ~/MEGAsync -file-browser-depth 3

Steps to reproduce

What behaviour you see

[1] 9930 segmentation fault (core dumped) rofi -show combi -combi-modi run#drun#file-browser -modi -sidebar-mode

What behaviour you expect to see

rofi quit without segfault

Debug

Digging into rofi and rofi-file-browser source code, I found that Modes are destroyed multiple times. I print from mode_int() and mode_destory() and get:

mode_init 0x5555555cbf40 display-combi
mode_init 0x5555555cba40 display-run
mode_init 0x5555555cbd00 display-drun
mode_init 0x7ffff7fcb3a0 display-file-browser
mode_init 0x5555555cba40 display-run
mode_init 0x5555555cbd00 display-drun
mode_init 0x7ffff7fcb3a0 display-file-browser
mode_destory 0x5555555cbf40 display-combi
mode_destory 0x5555555cba40 display-run
mode_destory 0x5555555cbd00 display-drun
mode_destory 0x7ffff7fcb3a0 display-file-browser
mode_destory 0x5555555cba40 display-run
mode_destory 0x5555555cbd00 display-drun
mode_destory 0x7ffff7fcb3a0 display-file-browser

(second column is Mode pointer, Note that run, drun, file-browser are initialized and destroyed twice.)

Because combi mode will call mode_init and mode_destory of its submodes, mode_destory will be called twice if -combi-modi and -modi overlaps.

rofi-file-browser-extended will segfault if its file_browser_destory called twice, and builtin modes does not.

georeth commented 4 years ago

First destory:

#0  0x00007ffff7fc44eb in file_browser_destroy (sw=0x5555555cbd00 <drun_mode>)
    at /home/georeth/Code/Source/rofi-file-browser-extended/src/filebrowser.c:67
#1  0x0000555555584704 in mode_destroy (mode=0x7ffff7fcb3a0 <mode>) at source/mode.c:57
#2  0x00005555555a539b in combi_mode_destroy (sw=0x5555555cbf40 <combi_mode>)
    at source/dialogs/combi.c:146
#3  0x0000555555584704 in mode_destroy (mode=0x5555555cbf40 <combi_mode>) at source/mode.c:57
#4  0x000055555557d5ce in cleanup () at source/rofi.c:426
#5  0x000055555557f03c in main (argc=17, argv=0x7fffffffdd28) at source/rofi.c:1077

second destory

#0  0x00007ffff7fc44eb in file_browser_destroy (sw=0x5555555cbd00 <drun_mode>)
    at /home/georeth/Code/Source/rofi-file-browser-extended/src/filebrowser.c:67
#1  0x0000555555584704 in mode_destroy (mode=0x7ffff7fcb3a0 <mode>) at source/mode.c:57
#2  0x000055555557d5ce in cleanup () at source/rofi.c:426
#3  0x000055555557f03c in main (argc=17, argv=0x7fffffffdd28) at source/rofi.c:1077
DaveDavenport commented 4 years ago

This looks more like a bug in rofi-file-browser-extended.

georeth commented 4 years ago

@DaveDavenport mode_init and mode_destroy should be implemented as idempotent operations?

DaveDavenport commented 4 years ago

It is expected that you handle this gracefully.

Most modi do this via the private data structure. init

 if ( mode_get_private_data ( sw ) != NULL ) {
     return TRUE;                             
 }                                            

destroy

DRunModePrivateData *rmpd = (DRunModePrivateData *) mode_get_private_data ( sw );
if ( rmpd != NULL ) {   
    ....
    mode_set_private_data ( sw, NULL );
}                                      
github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.