Tweaks to CensorshipCircumvention and also Meek cleanup fixes that were causing a crazy read loop on the stdout queue

cli/onionshare_cli/meek.py

@@ -20,8 +20,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import os
 import subprocess
 import time
-from queue import Queue, Empty
-from threading import Thread
 
 
 class Meek(object):
@@ -67,14 +65,6 @@ class Meek(object):
         Start the Meek Client and populate the SOCKS proxies dict
         for use with requests to the Tor Moat API.
         """
-        # Small method to read stdout from the subprocess.
-        # We use this to obtain the random port that Meek
-        # started on
-        def enqueue_output(out, queue):
-            for line in iter(out.readline, b""):
-                queue.put(line)
-            out.close()
-
         # Abort early if we can't find the Meek client
         if self.meek_client_file_path is None or not os.path.exists(
             self.meek_client_file_path
@@ -124,21 +114,8 @@ class Meek(object):
                 universal_newlines=True,
             )
 
-        # Queue up the stdout from the subprocess for polling later
-        q = Queue()
-        t = Thread(target=enqueue_output, args=(self.meek_proc.stdout, q))
-        t.daemon = True  # thread dies with the program
-        t.start()
-
-        while True:
-            # read stdout without blocking
-            try:
-                line = q.get_nowait()
-                self.common.log("Meek", "start", line.strip())
-            except Empty:
-                # no stdout yet?
-                pass
-            else:  # we got stdout
+        # Obtain the host and port that meek is running on
+        for line in iter(self.meek_proc.stdout.readline, b""):
             if "CMETHOD meek socks5" in line:
                 self.meek_host = line.split(" ")[3].split(":")[0]
                 self.meek_port = line.split(" ")[3].split(":")[1]
@@ -152,6 +129,7 @@ class Meek(object):
             if "CMETHOD-ERROR" in line:
                 self.cleanup()
                 raise MeekNotRunning()
+                break
 
         if self.meek_port:
             self.meek_proxies = {

desktop/src/onionshare/connection_tab.py

@@ -24,11 +24,15 @@ import random
 import time
 from PySide2 import QtCore, QtWidgets, QtGui
 
+from onionshare_cli.censorship import CensorshipCircumvention
+from onionshare_cli.meek import (
+    MeekNotRunning,
+    MeekNotFound,
+)
 from onionshare_cli.settings import Settings
 
 from . import strings
 from .gui_common import GuiCommon, ToggleCheckbox
-from .threads import CensorshipCircumventionThread
 from .tor_connection import TorConnectionWidget
 from .update_checker import UpdateThread
 from .widgets import Alert
@@ -158,14 +162,12 @@ class AutoConnectTab(QtWidgets.QWidget):
             "_got_bridges",
             "Got bridges. Trying to reconnect to Tor",
         )
-        self.active = False
         self.tor_con.show()
         self.tor_con.start(self.curr_settings)
 
     def _got_no_bridges(self):
         self.use_bridge_widget.progress.hide()
         self.use_bridge_widget.progress_label.hide()
-        self.active = False
         self.tor_con.fail.emit()
 
         Alert(
@@ -203,16 +205,39 @@ class AutoConnectTab(QtWidgets.QWidget):
         else:
             country = self.use_bridge_widget.country_code
 
-        t = CensorshipCircumventionThread(self.common, self.curr_settings, country)
-        t.progress_update.connect(self._censorship_progress_update)
-        t.got_bridges.connect(self._got_bridges)
-        t.got_no_bridges.connect(self._got_no_bridges)
-        t.start()
-        self.use_bridge_widget.progress.setValue(0)
-        self.active = True
-        while self.active:
-            time.sleep(0.1)
-            self.common.gui.qtapp.processEvents()
+        self._censorship_progress_update(
+            50, strings._("gui_autoconnect_circumventing_censorship_starting_meek")
+        )
+        try:
+            self.common.gui.meek.start()
+            self.censorship_circumvention = CensorshipCircumvention(
+                self.common, self.common.gui.meek
+            )
+            self._censorship_progress_update(
+                75, strings._("gui_autoconnect_circumventing_censorship_requesting_bridges")
+            )
+            bridge_settings = self.censorship_circumvention.request_settings(
+                country=country
+            )
+            self.common.gui.meek.cleanup()
+
+            if bridge_settings and self.censorship_circumvention.save_settings(
+                self.curr_settings, bridge_settings
+            ):
+                self._censorship_progress_update(
+                    100, strings._("gui_autoconnect_circumventing_censorship_got_bridges")
+                )
+                self._got_bridges()
+            else:
+                self._censorship_progress_update(
+                    100, strings._("gui_autoconnect_circumventing_censorship_no_bridges")
+                )
+                self._got_no_bridges()
+        except (
+            MeekNotRunning,
+            MeekNotFound,
+        ) as e:
+            self._got_no_bridges()
 
     def back_clicked(self):
         """

desktop/src/onionshare/threads.py

@@ -39,8 +39,6 @@ from onionshare_cli.onion import (
     PortNotAvailable,
 )
 
-from onionshare_cli.censorship import CensorshipCircumvention
-
 from . import strings
 
 
@@ -270,53 +268,3 @@ class OnionCleanupThread(QtCore.QThread):
     def run(self):
         self.common.log("OnionCleanupThread", "run")
         self.common.gui.onion.cleanup()
-
-
-class CensorshipCircumventionThread(QtCore.QThread):
-    progress_update = QtCore.Signal(int, str)
-    got_bridges = QtCore.Signal()
-    got_no_bridges = QtCore.Signal()
-
-    def __init__(self, common, settings, country):
-        super(CensorshipCircumventionThread, self).__init__()
-        self.common = common
-        self.progress_update.emit(
-            25,
-            strings._(
-                "gui_autoconnect_circumventing_censorship_starting_circumvention"
-            ),
-        )
-        self.common.log("CensorshipCircumventionThread", "__init__")
-        self.settings = settings
-        self.country = country
-
-    def run(self):
-        self.common.log("CensorshipCircumventionThread", "run")
-
-        self.progress_update.emit(
-            50, strings._("gui_autoconnect_circumventing_censorship_starting_meek")
-        )
-        self.common.gui.meek.start()
-        self.censorship_circumvention = CensorshipCircumvention(
-            self.common, self.common.gui.meek
-        )
-        self.progress_update.emit(
-            75, strings._("gui_autoconnect_circumventing_censorship_requesting_bridges")
-        )
-        bridge_settings = self.censorship_circumvention.request_settings(
-            country=self.country
-        )
-        self.common.gui.meek.cleanup()
-
-        if bridge_settings and self.censorship_circumvention.save_settings(
-            self.settings, bridge_settings
-        ):
-            self.progress_update.emit(
-                100, strings._("gui_autoconnect_circumventing_censorship_got_bridges")
-            )
-            self.got_bridges.emit()
-        else:
-            self.progress_update.emit(
-                100, strings._("gui_autoconnect_circumventing_censorship_no_bridges")
-            )
-            self.got_no_bridges.emit()