Red Hat Bugzilla – Bug 1368353
[NMCI] [abrt] [faf] NetworkManager: g_object_get_property(): /usr/bin/nmcli killed by 11
Last modified: 2017-08-01 05:17:07 EDT
This bug has been created based on an anonymous crash report requested by the package maintainer. Report URL: http://faf.lab.eng.brq.redhat.com/faf/reports/bthash/ea8d063089884094ab7de7a2158c22b74aec82ba/
no idea why this would happen... assigning to bgalvani :)
The crash happens when editing a connection with the 'nmcli connection edit', apparently because the @g_class field of a connection object is invalid in g_object_get_property(). I suspect this is caused by a race condition since the editor runs in its own thread different from the main thread of the GMainLoop, and GObjects are not thread-safe. I think the reason why the thread was added was to allow the use of readline() without blocking the processing of D-Bus events; but I'm not sure if this is safe since there is the possibility that the two threads access the same object concurrently. A possible fix would be to move the editor loop back to the main thread and switch to the readline alternate interface [1], which allows asynchronous operation. [1] https://cnswww.cns.cwru.edu/php/chet/readline/readline.html#SEC41
(In reply to Beniamino Galvani from comment #2) > I suspect this is caused by a race condition yeah, that sounds plausible (cannot say for sure). But surely, the editor thread is a bug and unexpected crashes may happen in edit mode.
Pushed branch bg/cli-readline-async-rh1368353 for review.
(In reply to Beniamino Galvani from comment #4) > Pushed branch bg/cli-readline-async-rh1368353 for review. please refer in the commit message to https://bugzilla.gnome.org/show_bug.cgi?id=732097 if (nmc_get_in_readline ()) { + g_print ("^C\n"); can we not print anything and just wrap the line? For the terminal, this is configurable via `stty -ctlecho`. It seems, we should not print this ourself. /* Add string to the history */ + if (rl_string && *rl_string) + add_history (rl_string); I think, if the user pressed CTRL+C, we should add the command to the history. let's drop nmc_set_in_readline() and have "g_return_val_if_fail (!in_readline, NULL);" at the preample of nmc_readline_helper(). I think signal_handler() must be async-signal-safe. g_message() is not allowed. I think g_unix_signal_add() is better. g_message() and other g_log() functions are not suitable inside nmcli, because they print a prefix and the logging severity. g_print() or g_printerr() should be used -- or better a wrapper like nmcli_print().
(In reply to Thomas Haller from comment #5) > (In reply to Beniamino Galvani from comment #4) > > Pushed branch bg/cli-readline-async-rh1368353 for review. > > please refer in the commit message to > https://bugzilla.gnome.org/show_bug.cgi?id=732097 ok > if (nmc_get_in_readline ()) { > + g_print ("^C\n"); > > can we not print anything and just wrap the line? > > For the terminal, this is configurable via `stty -ctlecho`. It seems, we > should not print this ourself. Mmh, doesn't seem to work with readline. I've restored the old way to print the ^C using "rl_echo_signal_char (SIGINT)". > /* Add string to the history */ > + if (rl_string && *rl_string) > + add_history (rl_string); > > I think, if the user pressed CTRL+C, we should add the command to the > history. Fixed. > let's drop nmc_set_in_readline() and have "g_return_val_if_fail > (!in_readline, NULL);" at the preample of nmc_readline_helper(). nmc_set_in_readline() is still needed to recognize whether SIGINT happens during a readline or not. > I think signal_handler() must be async-signal-safe. g_message() is not > allowed. > I think g_unix_signal_add() is better. Fixed. > g_message() and other g_log() functions are not suitable inside nmcli, > because they print a prefix and the logging severity. g_print() or > g_printerr() should be used -- or better a wrapper like nmcli_print(). Fine. Repushed branch bg/cli-readline-async-rh1368353.
lgtm now
*** Bug 1374635 has been marked as a duplicate of this bug. ***
lgtm
Merged to master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=36f561ead0191a914589fed284a05a9b3192293d
*** Bug 1379816 has been marked as a duplicate of this bug. ***
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2017:2299