[PATCH v2 1/3] station: Use separate msg holder for hidden connections

[PATCH v2 1/3] station: Use separate msg holder for hidden connections

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]

Introduce hidden_pending to keep reference to the dbus message object
while we wait for the scan results to be returned while trying to
connect to a hidden network. This simplifies the logic by separating it
into two independent logical units: scanning, connecting and eliminates
a possibility of a memory leak in the case when Network.Connect being
initiated while Station.ConnectHiddenNetwork is in progress.
---
 src/station.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/station.c b/src/station.c
index ca6a0b8e..583016ab 100644
--- a/src/station.c
+++ b/src/station.c
@@ -73,6 +73,7 @@ struct station {
 	struct l_hashmap *networks;
 	struct l_queue *networks_sorted;
 	struct l_dbus_message *connect_pending;
+	struct l_dbus_message *hidden_pending;
 	struct l_dbus_message *disconnect_pending;
 	struct l_dbus_message *scan_pending;
 	struct signal_agent *signal_agent;
@@ -2433,8 +2434,8 @@ static void station_hidden_network_scan_triggered(int err, void *user_data)
 	if (!err)
 		return;
 
-	dbus_pending_reply(&station->connect_pending,
-				dbus_error_failed(station->connect_pending));
+	dbus_pending_reply(&station->hidden_pending,
+				dbus_error_failed(station->hidden_pending));
 }
 
 static bool station_hidden_network_scan_results(int err,
@@ -2452,8 +2453,8 @@ static bool station_hidden_network_scan_results(int err,
 
 	l_debug("");
 
-	msg = station->connect_pending;
-	station->connect_pending = NULL;
+	msg = station->hidden_pending;
+	station->hidden_pending = NULL;
 
 	if (err) {
 		dbus_pending_reply(&msg, dbus_error_failed(msg));
@@ -2527,7 +2528,7 @@ static struct l_dbus_message *station_dbus_connect_hidden_network(
 
 	l_debug("");
 
-	if (station->connect_pending || station_is_busy(station))
+	if (station->hidden_pending || station_is_busy(station))
 		return dbus_error_busy(message);
 
 	if (!l_dbus_message_get_arguments(message, "s", &ssid))
@@ -2553,7 +2554,7 @@ static struct l_dbus_message *station_dbus_connect_hidden_network(
 	if (!station->hidden_network_scan_id)
 		return dbus_error_failed(message);
 
-	station->connect_pending = l_dbus_message_ref(message);
+	station->hidden_pending = l_dbus_message_ref(message);
 
 	return NULL;
 }
@@ -3132,6 +3133,10 @@ static void station_free(struct station *station)
 		dbus_pending_reply(&station->connect_pending,
 				dbus_error_aborted(station->connect_pending));
 
+	if (station->hidden_pending)
+		dbus_pending_reply(&station->hidden_pending,
+				dbus_error_aborted(station->hidden_pending));
+
 	if (station->disconnect_pending)
 		dbus_pending_reply(&station->disconnect_pending,
 			dbus_error_aborted(station->disconnect_pending));
-- 
2.13.6

[PATCH v2 2/3] auto-t: Add async API call for connect_hidden_network

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

---
 autotests/util/iwd.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index b9eeb0ff..ef1ea80e 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -482,6 +482,21 @@ class Device(IWDDBusAbstract):
                                error_handler=self._failure)
         self._wait_for_async_op()
 
+    def connect_hidden_network_async(self, name, reply_handler, error_handler):
+        '''Connect to a hidden network
+           Possible exception: BusyEx
+                               FailedEx
+                               InvalidArgumentsEx
+                               NotConfiguredEx
+                               NotConnectedEx
+                               NotFoundEx
+                               ServiceSetOverlapEx
+        '''
+        self._iface.ConnectHiddenNetwork(name,
+                               dbus_interface=IWD_STATION_INTERFACE,
+                               reply_handler=reply_handler,
+                               error_handler=error_handler)
+
     def start_adhoc(self, ssid, psk=None):
         self._prop_proxy.Set(IWD_DEVICE_INTERFACE, 'Mode', 'ad-hoc')
         self._adhoc_iface = dbus.Interface(self._bus.get_object(IWD_SERVICE,
-- 
2.13.6

[PATCH v2 3/3] auto-t: Validate Connect after ConnectHiddenNetwork

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 3769 bytes --]

---
 .../connect_after_hidden_connect_test.py           | 78 ++++++++++++++++++++++
 autotests/testHiddenNetworks/hw.conf               |  3 +-
 autotests/testHiddenNetworks/ssidSomeHidden.conf   |  9 +++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 autotests/testHiddenNetworks/connect_after_hidden_connect_test.py
 create mode 100644 autotests/testHiddenNetworks/ssidSomeHidden.conf

diff --git a/autotests/testHiddenNetworks/connect_after_hidden_connect_test.py b/autotests/testHiddenNetworks/connect_after_hidden_connect_test.py
new file mode 100644
index 00000000..478f1505
--- /dev/null
+++ b/autotests/testHiddenNetworks/connect_after_hidden_connect_test.py
@@ -0,0 +1,78 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+
+sys.path.append('../util')
+import iwd
+from iwd import IWD
+from iwd import PSKAgent
+import testutil
+import time
+
+class TestConnectionAfterHiddenNetwork(unittest.TestCase):
+    '''
+    Tries to reproduce a memory leak caused by the consecutive calls to
+    ConnectHiddenNetwork and Connect one after another.
+
+    '''
+    _ex = None
+    _done = False
+
+    def _success(self):
+        self._done = True
+
+    def _failure(self, ex):
+        self._done = True
+        self._ex = ex
+
+    def test_connection(self):
+        wd = IWD(True, '/tmp')
+
+        psk_agent = PSKAgent("secret123")
+        wd.register_psk_agent(psk_agent)
+
+        device = wd.list_devices(1)[0]
+        ordered_network = device.get_ordered_network('ssidOpen',
+                                                     scan_if_needed=True)
+
+        device.connect_hidden_network_async(name='ssidSomeHidden',
+                                                  reply_handler = self._success,
+                                                  error_handler = self._failure)
+
+        ordered_network.network_object.connect()
+
+        condition = 'obj.state == DeviceState.connected'
+        wd.wait_for_object_condition(device, condition)
+
+        condition = 'obj.connected_network is not None'
+        wd.wait_for_object_condition(device, condition)
+
+        testutil.test_iface_operstate(device.name)
+        device.disconnect()
+
+        condition = 'obj.state == DeviceState.disconnected'
+        wd.wait_for_object_condition(device, condition)
+
+        wd.unregister_psk_agent(psk_agent)
+
+        IWD.clear_storage()
+
+        while not self._done:
+            time.sleep(.300)
+
+        if self._ex is not None:
+            if self._ex.get_dbus_name() != 'net.connman.iwd.Failed':
+                raise self._ex
+
+
+    @classmethod
+    def setUpClass(cls):
+        pass
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
diff --git a/autotests/testHiddenNetworks/hw.conf b/autotests/testHiddenNetworks/hw.conf
index edd061a8..7147e4be 100644
--- a/autotests/testHiddenNetworks/hw.conf
+++ b/autotests/testHiddenNetworks/hw.conf
@@ -1,5 +1,5 @@
 [SETUP]
-num_radios=6
+num_radios=7
 start_iwd=0
 max_test_exec_interval_sec=160
 tmpfs_extra_stuff=main.conf
@@ -10,3 +10,4 @@ rad1=ssidHiddenWPA.conf
 rad2=ssidOverlap1.conf
 rad3=ssidOverlap2.conf
 rad4=ssidOpen.conf
+rad5=ssidSomeHidden.conf
diff --git a/autotests/testHiddenNetworks/ssidSomeHidden.conf b/autotests/testHiddenNetworks/ssidSomeHidden.conf
new file mode 100644
index 00000000..42c69b3a
--- /dev/null
+++ b/autotests/testHiddenNetworks/ssidSomeHidden.conf
@@ -0,0 +1,9 @@
+hw_mode=g
+channel=1
+ssid=ssidSomeHidden
+
+wpa=1
+wpa_pairwise=TKIP
+wpa_passphrase=secret123
+
+ignore_broadcast_ssid=1
-- 
2.13.6

Re: [PATCH v2 1/3] station: Use separate msg holder for hidden connections

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Hi Tim,

On 6/9/20 8:22 PM, Tim Kourt wrote:
> Introduce hidden_pending to keep reference to the dbus message object
> while we wait for the scan results to be returned while trying to
> connect to a hidden network. This simplifies the logic by separating it
> into two independent logical units: scanning, connecting and eliminates
> a possibility of a memory leak in the case when Network.Connect being
> initiated while Station.ConnectHiddenNetwork is in progress.
> ---
>   src/station.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 

All applied, thanks.

Regards,
-Denis