Commit 61b4f31d authored by Alfonso Sánchez-Beato's avatar Alfonso Sánchez-Beato Committed by Thomas Haller

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 fe090c34, 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]: <warn>  (ttyACM1): could not re-acquire serial port lock: (5) Input/output error
ModemManager[2158]: <warn>  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 a251712a)
(cherry picked from commit 3caa0657)
parent 3a8cab97
Pipeline #42656 failed with stage
in 127 minutes and 17 seconds
...@@ -49,7 +49,7 @@ char pppd_version[] = VERSION; ...@@ -49,7 +49,7 @@ char pppd_version[] = VERSION;
static GDBusProxy *proxy = NULL; static GDBusProxy *proxy = NULL;
static void static void
nm_phasechange (void *data, int arg) nm_phasechange (int arg)
{ {
NMPPPStatus ppp_status = NM_PPP_STATUS_UNKNOWN; NMPPPStatus ppp_status = NM_PPP_STATUS_UNKNOWN;
char new_name[IF_NAMESIZE]; char new_name[IF_NAMESIZE];
...@@ -151,6 +151,16 @@ nm_phasechange (void *data, int arg) ...@@ -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 static void
nm_ip_up (void *data, int arg) nm_ip_up (void *data, int arg)
{ {
...@@ -165,7 +175,7 @@ nm_ip_up (void *data, int arg) ...@@ -165,7 +175,7 @@ nm_ip_up (void *data, int arg)
if (!opts.ouraddr) { if (!opts.ouraddr) {
g_warning ("nm-ppp-plugin: (%s): didn't receive an internal IP from pppd!", __func__); 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; return;
} }
...@@ -357,6 +367,11 @@ nm_exit_notify (void *data, int arg) ...@@ -357,6 +367,11 @@ nm_exit_notify (void *data, int arg)
{ {
g_return_if_fail (G_IS_DBUS_PROXY (proxy)); 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_message ("nm-ppp-plugin: (%s): cleaning up", __func__);
g_object_unref (proxy); g_object_unref (proxy);
...@@ -424,7 +439,7 @@ plugin_init (void) ...@@ -424,7 +439,7 @@ plugin_init (void)
pap_passwd_hook = get_credentials; pap_passwd_hook = get_credentials;
pap_check_hook = get_pap_check; 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 (&ip_up_notifier, nm_ip_up, NULL);
add_notifier (&exitnotify, nm_exit_notify, proxy); add_notifier (&exitnotify, nm_exit_notify, proxy);
add_ip6_notifier (); add_ip6_notifier ();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment