From 3caa065709f3f3aec0c948dd25c282e88fa10ded Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= Date: Thu, 13 Jun 2019 18:23:21 +0200 Subject: [PATCH] core/pppd-plugin: wait to recover port settings before notifying death pppd restores the previous settings for the serial port it uses right before exiting. It is especially important to do so because otherwise ModemManager is not able to recover the port as it can receive a hangup event from the port due to CLOCAL not being restored. However, there is currently a race condition that produces this issue. This is because when PHASE_DEAD is notified, pppd still has not restored the port settings - it does that a bit later, in the die() function. This patch delays notifying PHASE_DEAD until when the exitnotify() hook is called by pppd: when this happens the port settings have already been restored. There were previously efforts to fix this in commit fe090c34b724, so PHASE_DEAD was used instead of PHASE_DISCONNECT to notify MM that the port was disconnected, but that still early to ensure that the port settings are restored. The MM traces seen when the bug is triggered are: ModemManager[2158]: (ttyACM1): could not re-acquire serial port lock: (5) Input/output error ModemManager[2158]: Couldn't load Operator Code: 'Cannot run sequence: 'Could not open serial device ttyACM1: it has been forced close' https://mail.gnome.org/archives/networkmanager-list/2019-June/msg00014.html (cherry picked from commit a251712a729f676c9c2d38e469e4f88e250229ba) --- src/ppp/nm-pppd-plugin.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ppp/nm-pppd-plugin.c b/src/ppp/nm-pppd-plugin.c index a8d6749a43..c40ad9f565 100644 --- a/src/ppp/nm-pppd-plugin.c +++ b/src/ppp/nm-pppd-plugin.c @@ -49,7 +49,7 @@ char pppd_version[] = VERSION; static GDBusProxy *proxy = NULL; static void -nm_phasechange (void *data, int arg) +nm_phasechange (int arg) { NMPPPStatus ppp_status = NM_PPP_STATUS_UNKNOWN; char new_name[IF_NAMESIZE]; @@ -151,6 +151,16 @@ nm_phasechange (void *data, int arg) } } +static void +nm_phasechange_hook (void *data, int arg) +{ + /* We send the nofication in exitnotify instead */ + if (arg == PHASE_DEAD) + return; + + nm_phasechange (arg); +} + static void nm_ip_up (void *data, int arg) { @@ -165,7 +175,7 @@ nm_ip_up (void *data, int arg) if (!opts.ouraddr) { g_warning ("nm-ppp-plugin: (%s): didn't receive an internal IP from pppd!", __func__); - nm_phasechange (NULL, PHASE_DEAD); + nm_phasechange (PHASE_DEAD); return; } @@ -357,6 +367,11 @@ nm_exit_notify (void *data, int arg) { g_return_if_fail (G_IS_DBUS_PROXY (proxy)); + /* We wait until this point to notify dead phase to make sure that + * the serial port has recovered already its original settings. + */ + nm_phasechange (PHASE_DEAD); + g_message ("nm-ppp-plugin: (%s): cleaning up", __func__); g_object_unref (proxy); @@ -424,7 +439,7 @@ plugin_init (void) pap_passwd_hook = get_credentials; pap_check_hook = get_pap_check; - add_notifier (&phasechange, nm_phasechange, NULL); + add_notifier (&phasechange, nm_phasechange_hook, NULL); add_notifier (&ip_up_notifier, nm_ip_up, NULL); add_notifier (&exitnotify, nm_exit_notify, proxy); add_ip6_notifier ();