refactor create_client to be async (works to run, some unit-test fails still)

src/allmydata/client.py

@@ -17,7 +17,7 @@ from allmydata.immutable.offloaded import Helper
 from allmydata.control import ControlServer
 from allmydata.introducer.client import IntroducerClient
 from allmydata.util import (hashutil, base32, pollmixin, log, keyutil, idlib,
-                            yamlutil)
+                            yamlutil, fileutil)
 from allmydata.util.encodingutil import (get_filesystem_encoding,
                                          from_utf8_or_none)
 from allmydata.util.abbreviate import parse_abbreviated_size
@@ -178,6 +178,25 @@ def read_config(basedir, portnumfile, generated_files=[]):
 
 
 #@defer.inlineCallbacks
+def create_client(basedir=u"."
+    node.create_node_dir(basedir, CLIENT_README)
+    config = read_config(basedir, u"client.port")
+
+    if _client_factory is None:
+        _client_factory = _Client
+
+    #defer.returnValue(
+    return _client_factory(
+            config,
+=======
+PRIV_README="""
+This directory contains files which contain private data for the Tahoe node,
+such as private keys.  On Unix-like systems, the permissions on this directory
+are set to disallow users other than its owner from reading the contents of
+the files.   See the 'configuration.rst' documentation file for details."""
+
+
+@defer.inlineCallbacks
 def create_client(basedir=u".", _client_factory=None):
     """
     Creates a new client instance (a subclass of Node).
@@ -191,17 +210,41 @@ def create_client(basedir=u".", _client_factory=None):
     :returns: :class:`allmydata.client._Client` instance (or whatever
         `_client_factory` returns)
     """
+    from allmydata.node import read_config, create_connection_handlers, create_tub_options
+    from allmydata.node import create_main_tub, create_control_tub, create_tub
+
+    # should we check for this directory existing first? (this used to
+    # be in Node's constructor)
     node.create_node_dir(basedir, CLIENT_README)
     config = read_config(basedir, u"client.port")
 
     if _client_factory is None:
         _client_factory = _Client
 
-    #defer.returnValue(
+    default_connection_handlers, foolscap_connection_handlers = create_connection_handlers(reactor, basedir, config)
-    return _client_factory(
+    tub_options = create_tub_options(config)
-            config,
+
+    yield
+
+    i2p_provider = None
+    tor_provider = None
+    reveal_ip = True # XXX FIXME
+    main_tub, is_listening = create_main_tub(
+        basedir, config, tub_options, default_connection_handlers,
+        foolscap_connection_handlers, reveal_ip=reveal_ip,
+    )
+    control_tub = create_control_tub()
+    defer.returnValue(
+        _Client(
+            config,
+            main_tub,
+            control_tub,
+            i2p_provider,
+            tor_provider,
+            basedir,
+            tub_is_listening=is_listening,
+        )
     )
-    #)
 
 
 @implementer(IStatsProducer)
@@ -226,8 +269,8 @@ class _Client(node.Node, pollmixin.PollMixin):
                                    "max_segment_size": 128*KiB,
                                    }
 
-    def __init__(self, config):
+    def __init__(self, config, main_tub, control_tub, i2p_provider, tor_provider, basedir, tub_is_listening):
-        node.Node.__init__(self, config)
+        node.Node.__init__(self, config, main_tub, control_tub, i2p_provider, tor_provider, basedir, tub_is_listening)
         # All tub.registerReference must happen *after* we upcall, since
         # that's what does tub.setLocation()
         self._magic_folders = dict()
@@ -496,8 +539,22 @@ class _Client(node.Node, pollmixin.PollMixin):
         # (and everybody else who wants to use storage servers)
         ps = self.config.get_config("client", "peers.preferred", "").split(",")
         preferred_peers = tuple([p.strip() for p in ps if p != ""])
-        sb = storage_client.StorageFarmBroker(permute_peers=True,
+
-                                              tub_maker=self._create_tub,
+        from allmydata.node import create_tub, create_tub_options
+
+        def tub_creator(handler_overrides={}, **kwargs):
+            tub_options = create_tub_options(self.config)
+            return create_tub(
+                tub_options,
+                self._default_connection_handlers,
+                self._foolscap_connection_handlers,
+                handler_overrides=handler_overrides,
+                **kwargs
+            )
+
+        sb = storage_client.StorageFarmBroker(
+            permute_peers=True,
+            tub_maker=tub_creator,
             preferred_peers=preferred_peers,
         )
         self.storage_broker = sb

src/allmydata/node.py

@@ -400,119 +400,10 @@ class _Config(object):
         return False
 
 
-
+def create_tub_options(config):
-class Node(service.MultiService):
+    # XXX this is code moved from Node -- but why are some options
-    """
+    # camelCase and some snake_case? can we FIXME?
-    This class implements common functionality of both Client nodes and Introducer nodes.
+    tub_options = {
-    """
-    NODETYPE = "unknown NODETYPE"
-    CERTFILE = "node.pem"
-    GENERATED_FILES = []
-
-    def __init__(self, config):
-        """
-        Initialize the node with the given configuration. Its base directory
-        is the current directory by default.
-        """
-        service.MultiService.__init__(self)
-
-        self.config = config
-        self.get_config = config.get_config # XXX stopgap
-        self.nickname = config.nickname # XXX stopgap
-
-        self.init_tempdir()
-        self.check_privacy()
-
-        self.create_log_tub()
-        self.logSource = "Node"
-        self.setup_logging()
-
-        self.create_i2p_provider()
-        self.create_tor_provider()
-        self.init_connections()
-        self.set_tub_options()
-        self.create_main_tub()
-        self.create_control_tub()
-
-        self.log("Node constructed. " + get_package_versions_string())
-        iputil.increase_rlimits()
-
-    def init_tempdir(self):
-        """
-        Initialize/create a directory for temporary files.
-        """
-        tempdir_config = self.config.get_config("node", "tempdir", "tmp").decode('utf-8')
-        tempdir = self.config.get_config_path(tempdir_config)
-        if not os.path.exists(tempdir):
-            fileutil.make_dirs(tempdir)
-        tempfile.tempdir = tempdir
-        # this should cause twisted.web.http (which uses
-        # tempfile.TemporaryFile) to put large request bodies in the given
-        # directory. Without this, the default temp dir is usually /tmp/,
-        # which is frequently too small.
-        temp_fd, test_name = tempfile.mkstemp()
-        _assert(os.path.dirname(test_name) == tempdir, test_name, tempdir)
-        os.close(temp_fd)  # avoid leak of unneeded fd
-
-    def check_privacy(self):
-        self._reveal_ip = self.config.get_config("node", "reveal-IP-address", True,
-                                                 boolean=True)
-    def create_i2p_provider(self):
-        self._i2p_provider = i2p_provider.Provider(self.config, reactor)
-        self._i2p_provider.check_dest_config()
-        self._i2p_provider.setServiceParent(self)
-
-    def create_tor_provider(self):
-        self._tor_provider = tor_provider.Provider(self.config, reactor)
-        self._tor_provider.check_onion_config()
-        self._tor_provider.setServiceParent(self)
-
-    def _make_tcp_handler(self):
-        # this is always available
-        from foolscap.connections.tcp import default
-        return default()
-
-    def _make_tor_handler(self):
-        return self._tor_provider.get_tor_handler()
-
-    def _make_i2p_handler(self):
-        return self._i2p_provider.get_i2p_handler()
-
-    def init_connections(self):
-        # We store handlers for everything. None means we were unable to
-        # create that handler, so hints which want it will be ignored.
-        handlers = self._foolscap_connection_handlers = {
-            "tcp": self._make_tcp_handler(),
-            "tor": self._make_tor_handler(),
-            "i2p": self._make_i2p_handler(),
-            }
-        self.log(format="built Foolscap connection handlers for: %(known_handlers)s",
-                 known_handlers=sorted([k for k, v in handlers.items() if v]),
-                 facility="tahoe.node", umid="PuLh8g")
-
-        # then we remember the default mappings from tahoe.cfg
-        self._default_connection_handlers = {"tor": "tor", "i2p": "i2p"}
-        tcp_handler_name = self.config.get_config("connections", "tcp", "tcp").lower()
-        if tcp_handler_name == "disabled":
-            self._default_connection_handlers["tcp"] = None
-        else:
-            if tcp_handler_name not in handlers:
-                raise ValueError("'tahoe.cfg [connections] tcp='"
-                                 " uses unknown handler type '%s'"
-                                 % tcp_handler_name)
-            if not handlers[tcp_handler_name]:
-                raise ValueError("'tahoe.cfg [connections] tcp=' uses "
-                                 "unavailable/unimportable handler type '%s'. "
-                                 "Please pip install tahoe-lafs[%s] to fix."
-                                 % (tcp_handler_name, tcp_handler_name))
-            self._default_connection_handlers["tcp"] = tcp_handler_name
-
-        if not self._reveal_ip:
-            if self._default_connection_handlers.get("tcp") == "tcp":
-                raise PrivacyError("tcp = tcp, must be set to 'tor' or 'disabled'")
-
-    def set_tub_options(self):
-        self.tub_options = {
         "logLocalFailures": True,
         "logRemoteFailures": True,
         "expose-remote-exception-types": False,
@@ -520,35 +411,135 @@ class Node(service.MultiService):
     }
 
     # see #521 for a discussion of how to pick these timeout values.
-        keepalive_timeout_s = self.config.get_config("node", "timeout.keepalive", "")
+    keepalive_timeout_s = config.get_config("node", "timeout.keepalive", "")
     if keepalive_timeout_s:
-            self.tub_options["keepaliveTimeout"] = int(keepalive_timeout_s)
+        tub_options["keepaliveTimeout"] = int(keepalive_timeout_s)
-        disconnect_timeout_s = self.config.get_config("node", "timeout.disconnect", "")
+    disconnect_timeout_s = config.get_config("node", "timeout.disconnect", "")
     if disconnect_timeout_s:
         # N.B.: this is in seconds, so use "1800" to get 30min
-            self.tub_options["disconnectTimeout"] = int(disconnect_timeout_s)
+        tub_options["disconnectTimeout"] = int(disconnect_timeout_s)
+    return tub_options
 
-    def _create_tub(self, handler_overrides={}, **kwargs):
+
+def create_i2p_provider(reactor, basedir, config):
+    provider = i2p_provider.Provider(basedir, config, reactor)
+    provider.check_dest_config()
+    #self._i2p_provider.setServiceParent(self)
+    return provider
+
+
+def create_tor_provider(reactor, basedir, config):
+    provider = tor_provider.Provider(basedir, config, reactor)
+    provider.check_onion_config()
+    ##self._tor_provider.setServiceParent(self)
+    return provider
+
+
+def _make_tcp_handler():
+    # this is always available
+    from foolscap.connections.tcp import default
+    return default()
+
+
+# XXX shouldn't need this
+def _make_tor_handler(tor_provider):
+    tor_provider.get_tor_handler()
+
+
+# XXX shouldn't need this
+def _make_i2p_handler(i2p_provider):
+    i2p_provider.get_i2p_handler()
+
+
+def create_connection_handlers(reactor, basedir, config):
+    """
+    :returns: 2-tuple of default_connection_handlers, foolscap_connection_handlers
+    """
+    reveal_ip = config.get_config("node", "reveal-IP-address", True, boolean=True)
+    i2p_provider = create_i2p_provider(reactor, basedir, config)
+    tor_provider = create_tor_provider(reactor, basedir, config)
+
+    # We store handlers for everything. None means we were unable to
+    # create that handler, so hints which want it will be ignored.
+    handlers = foolscap_connection_handlers = {
+        "tcp": _make_tcp_handler(),
+        "tor": _make_tor_handler(tor_provider),
+        "i2p": _make_i2p_handler(i2p_provider),
+        }
+    log.msg(
+        format="built Foolscap connection handlers for: %(known_handlers)s",
+        known_handlers=sorted([k for k,v in handlers.items() if v]),
+        facility="tahoe.node",
+        umid="PuLh8g",
+    )
+
+    # then we remember the default mappings from tahoe.cfg
+    default_connection_handlers = {"tor": "tor", "i2p": "i2p"}
+    tcp_handler_name = config.get_config("connections", "tcp", "tcp").lower()
+    if tcp_handler_name == "disabled":
+        default_connection_handlers["tcp"] = None
+    else:
+        if tcp_handler_name not in handlers:
+            raise ValueError(
+                "'tahoe.cfg [connections] tcp=' uses "
+                "unknown handler type '{}'".format(
+                    tcp_handler_name
+                )
+            )
+        if not handlers[tcp_handler_name]:
+            raise ValueError(
+                "'tahoe.cfg [connections] tcp=' uses "
+                "unavailable/unimportable handler type '{}'. "
+                "Please pip install tahoe-lafs[{}] to fix.".format(
+                    tcp_handler_name,
+                    tcp_handler_name,
+                )
+            )
+        default_connection_handlers["tcp"] = tcp_handler_name
+
+    if not reveal_ip:
+        if default_connection_handlers.get("tcp") == "tcp":
+            raise PrivacyError("tcp = tcp, must be set to 'tor' or 'disabled'")
+    return default_connection_handlers, foolscap_connection_handlers
+
+
+
+def create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,
+               handler_overrides={}, **kwargs):
     # Create a Tub with the right options and handlers. It will be
     # ephemeral unless the caller provides certFile=
     tub = Tub(**kwargs)
-        for (name, value) in self.tub_options.items():
+    for (name, value) in tub_options.items():
         tub.setOption(name, value)
-        handlers = self._default_connection_handlers.copy()
+    handlers = default_connection_handlers.copy()
     handlers.update(handler_overrides)
     tub.removeAllConnectionHintHandlers()
     for hint_type, handler_name in handlers.items():
-            handler = self._foolscap_connection_handlers.get(handler_name)
+        handler = foolscap_connection_handlers.get(handler_name)
         if handler:
             tub.addConnectionHintHandler(hint_type, handler)
     return tub
 
-    def _convert_tub_port(self, s):
+
+def _write_config(basedir, name, value, mode="w"):
+    """
+    Write a string to a config file.
+    """
+    fn = os.path.join(basedir, name)
+    try:
+        fileutil.write(fn, value, mode)
+    except EnvironmentError, e:
+        log.msg("Unable to write config file '{}'".format(fn))
+        log.err(e)
+
+
+def _convert_tub_port(s):
     if re.search(r'^\d+$', s):
-            return "tcp:%d" % int(s)
+        return "tcp:{}".format(int(s))
     return s
 
-    def get_tub_portlocation(self, cfg_tubport, cfg_location):
+
+def _tub_portlocation(config, cfg_tubport, cfg_location, reveal_ip=True):
     # return None, or tuple of (port, location)
 
     tubport_disabled = False
@@ -578,15 +569,15 @@ class Node(service.MultiService):
         # For 'tub.port', tahoe.cfg overrides the individual file on
         # disk. So only read self._portnumfile if tahoe.cfg doesn't
         # provide a value.
-            if os.path.exists(self.config.portnum_fname):
+        if os.path.exists(config.portnum_fname):
-                file_tubport = fileutil.read(self.config.portnum_fname).strip()
+            file_tubport = fileutil.read(config.portnum_fname).strip()
-                tubport = self._convert_tub_port(file_tubport)
+            tubport = _convert_tub_port(file_tubport)
         else:
             tubport = "tcp:%d" % iputil.allocate_tcp_port()
-                fileutil.write_atomically(self.config.portnum_fname, tubport + "\n",
+            fileutil.write_atomically(config.portnum_fname, tubport + "\n",
                                       mode="")
     else:
-            tubport = self._convert_tub_port(cfg_tubport)
+        tubport = _convert_tub_port(cfg_tubport)
 
     if cfg_location is None:
         cfg_location = "AUTO"
@@ -596,7 +587,7 @@ class Node(service.MultiService):
     # addresses. Don't probe for local addresses unless necessary.
     split_location = cfg_location.split(",")
     if "AUTO" in split_location:
-            if not self._reveal_ip:
+        if not reveal_ip:
             raise PrivacyError("tub.location uses AUTO")
         local_addresses = iputil.get_local_addresses_sync()
         # tubport must be like "tcp:12345" or "tcp:12345:morestuff"
@@ -607,7 +598,7 @@ class Node(service.MultiService):
             new_locations.extend(["tcp:%s:%d" % (ip, local_portnum)
                                   for ip in local_addresses])
         else:
-                if not self._reveal_ip:
+            if not reveal_ip:
                 # Legacy hints are "host:port". We use Foolscap's utility
                 # function to convert all hints into the modern format
                 # ("tcp:host:port") because that's what the receiving
@@ -624,16 +615,16 @@ class Node(service.MultiService):
 
     return tubport, location
 
-    def create_main_tub(self):
-        certfile = self.config.get_private_path(self.CERTFILE)
-        self.tub = self._create_tub(certFile=certfile)
 
-        self.nodeid = b32decode(self.tub.tubID.upper()) # binary format
+def create_main_tub(basedir, config, tub_options, default_connection_handlers,
-        self.config.write_config_file("my_nodeid", b32encode(self.nodeid).lower() + "\n")
+                    foolscap_connection_handlers, handler_overrides={}, reveal_ip=True):
-        self.short_nodeid = b32encode(self.nodeid).lower()[:8] # for printing
+    certfile = os.path.join(basedir, "private", "node.pem")  # FIXME "node.pem" was the CERTFILE option/thing
-        cfg_tubport = self.config.get_config("node", "tub.port", None)
+    tub = create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,
-        cfg_location = self.config.get_config("node", "tub.location", None)
+                     handler_overrides=handler_overrides, certFile=certfile)
-        portlocation = self.get_tub_portlocation(cfg_tubport, cfg_location)
+
+    cfg_tubport = config.get_config("node", "tub.port", None)
+    cfg_location = config.get_config("node", "tub.location", None)
+    portlocation = _tub_portlocation(config, cfg_tubport, cfg_location, reveal_ip)
     if portlocation:
         tubport, location = portlocation
         for port in tubport.split(","):
@@ -650,29 +641,99 @@ class Node(service.MultiService):
                 port_or_endpoint = self._tor_provider.get_listener()
             else:
                 port_or_endpoint = port
-                self.tub.listenOn(port_or_endpoint)
+            tub.listenOn(port_or_endpoint)
-            self.tub.setLocation(location)
+        tub.setLocation(location)
-            self._tub_is_listening = True
+        tub_is_listening = True
-            self.log("Tub location set to %s" % (location,))
+        log.msg("Tub location set to %s" % (location,))
         # the Tub is now ready for tub.registerReference()
     else:
-            self._tub_is_listening = False
+        tub_is_listening = False
-            self.log("Tub is not listening")
+        log.msg("Tub is not listening")
 
-        self.tub.setServiceParent(self)
+    # XXX can we get rid of the tub_is_listening part?
+    return tub, tub_is_listening
 
-    def create_control_tub(self):
+
+def create_control_tub():
     # the control port uses a localhost-only ephemeral Tub, with no
     # control over the listening port or location
-        self.control_tub = Tub()
+    control_tub = Tub()
     portnum = iputil.allocate_tcp_port()
     port = "tcp:%d:interface=127.0.0.1" % portnum
     location = "tcp:127.0.0.1:%d" % portnum
-        self.control_tub.listenOn(port)
+    control_tub.listenOn(port)
-        self.control_tub.setLocation(location)
+    control_tub.setLocation(location)
-        self.log("Control Tub location set to %s" % (location,))
+    log.msg("Control Tub location set to %s" % (location,))
-        self.control_tub.setServiceParent(self)
+    return control_tub
 
+
+
+class Node(service.MultiService):
+    """
+    This class implements common functionality of both Client nodes and Introducer nodes.
+    """
+    NODETYPE = "unknown NODETYPE"
+    CERTFILE = "node.pem"
+    GENERATED_FILES = []
+
+    def __init__(self, config, main_tub, control_tub, i2p_provider, tor_provider, basedir, tub_is_listening):
+        """
+        Initialize the node with the given configuration. Its base directory
+        is the current directory by default.
+        """
+        service.MultiService.__init__(self)
+
+        self._tub_is_listening = tub_is_listening  # holdover; do we really need this?
+        self.config = config
+        self.get_config = config.get_config # XXX stopgap
+        self.nickname = config.nickname # XXX stopgap
+
+        self.init_tempdir()
+
+        self.create_log_tub()
+        self.logSource = "Node"
+        self.setup_logging()
+
+        # XXX do we need to save these? or does just "create_client"
+        # need them? (note: look in client.py also!)
+        # (client.py DOES use them in init_client_storage_broker, but
+        # we'll want to pull that out as well...so FIXME later)
+        self._default_connection_handlers, self._foolscap_connection_handlers = create_connection_handlers(reactor, basedir, config)
+
+        self.tub = main_tub
+        if self.tub is not None:
+            self.nodeid = b32decode(self.tub.tubID.upper()) # binary format
+            self.short_nodeid = b32encode(self.nodeid).lower()[:8] # for printing
+            self.write_config("my_nodeid", b32encode(self.nodeid).lower() + "\n")
+            self.tub.setServiceParent(self)  # is this okay in __init__?
+        else:
+            self.nodeid = self.short_nodeid = None
+
+        self.control_tub = control_tub
+        if self.control_tub is not None:
+            self.control_tub.setServiceParent(self)  # is this okay in __init__?
+
+        self.log("Node constructed. " + get_package_versions_string())
+        iputil.increase_rlimits()
+
+    def init_tempdir(self):
+        """
+        Initialize/create a directory for temporary files.
+        """
+        tempdir_config = self.config.get_config("node", "tempdir", "tmp").decode('utf-8')
+        tempdir = self.config.get_config_path(tempdir_config)
+        if not os.path.exists(tempdir):
+            fileutil.make_dirs(tempdir)
+        tempfile.tempdir = tempdir
+        # this should cause twisted.web.http (which uses
+        # tempfile.TemporaryFile) to put large request bodies in the given
+        # directory. Without this, the default temp dir is usually /tmp/,
+        # which is frequently too small.
+        temp_fd, test_name = tempfile.mkstemp()
+        _assert(os.path.dirname(test_name) == tempdir, test_name, tempdir)
+        os.close(temp_fd)  # avoid leak of unneeded fd
+
+    # XXX probably want to pull this outside too?
     def create_log_tub(self):
         # The logport uses a localhost-only ephemeral Tub, with no control
         # over the listening port or location. This might change if we
@@ -688,6 +749,12 @@ class Node(service.MultiService):
         self.log("Log Tub location set to %s" % (location,))
         self.log_tub.setServiceParent(self)
 
+    # XXX this should be deprecated; no reason for it to be a method;
+    # use _write_config() instead
+    def write_config(self, name, value, mode="w"):
+        """Write a string to a config file."""
+        _write_config(self.basedir, name, value, mode=mode)
+
     def startService(self):
         # Note: this class can be started and stopped at most once.
         self.log("Node.startService")

src/allmydata/scripts/tahoe_daemonize.py

@@ -4,12 +4,16 @@ from allmydata.scripts.common import BasedirOptions
 from twisted.scripts import twistd
 from twisted.python import usage
 from twisted.python.reflect import namedAny
+from twisted.internet.defer import maybeDeferred, fail
+from twisted.application.service import Service
+
 from allmydata.scripts.default_nodedir import _default_nodedir
 from allmydata.util import fileutil
 from allmydata.node import read_config
 from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path
 from allmydata.util.configutil import UnknownConfigError
 from twisted.application.service import Service
+from allmydata.util.deferredutil import HookMixin
 
 
 def get_pidfile(basedir):
@@ -102,22 +106,33 @@ class MyTwistdConfig(twistd.ServerOptions):
     subCommands = [("DaemonizeTahoeNode", None, usage.Options, "node")]
 
 
-class DaemonizeTheRealService(Service):
+class DaemonizeTheRealService(Service, HookMixin):
+    """
+    this HookMixin should really be a helper; our hooks:
+
+    - 'running': triggered when startup has completed; it triggers
+        with None of successful or a Failure otherwise.
+    """
 
     def __init__(self, nodetype, basedir, options):
+        super(DaemonizeTheRealService, self).__init__()
         self.nodetype = nodetype
         self.basedir = basedir
+        # setup for HookMixin
+        self._hooks = {
+            "running": None,
+        }
 
     def startService(self):
 
         def key_generator_removed():
-            raise ValueError("key-generator support removed, see #2783")
+            return fail(ValueError("key-generator support removed, see #2783"))
 
         def start():
             node_to_instance = {
-                u"client": lambda: namedAny("allmydata.client.create_client")(self.basedir),
+                u"client": lambda: maybeDeferred(namedAny("allmydata.client.create_client"), self.basedir),
-                u"introducer": lambda: namedAny("allmydata.introducer.server.create_introducer")(self.basedir),
+                u"introducer": lambda: maybeDeferred(namedAny("allmydata.introducer.server.create_introducer"), self.basedir),
-                u"stats-gatherer": lambda: namedAny("allmydata.stats.StatsGathererService")(read_config(self.basedir, None), self.basedir, verbose=True),
+                u"stats-gatherer": lambda: maybeDeferred(namedAny("allmydata.stats.StatsGathererService"), read_config(self.basedir, None), self.basedir, verbose=True),
                 u"key-generator": key_generator_removed,
             }
 
@@ -126,15 +141,27 @@ class DaemonizeTheRealService(Service):
             except KeyError:
                 raise ValueError("unknown nodetype %s" % self.nodetype)
 
-            try:
+            def handle_config_error(fail):
-                srv = service_factory()
+                fail.trap(UnknownConfigError)
-                srv.setServiceParent(self.parent)
+                sys.stderr.write("\nConfiguration error:\n{}\n\n".format(fail.value))
-            except UnknownConfigError as e:
-                sys.stderr.write("\nConfiguration error:\n{}\n\n".format(e))
                 reactor.stop()
+                return
+
+            d = service_factory()
+
+            def created(srv):
+                srv.setServiceParent(self.parent)
+            d.addCallback(created)
+            d.addErrback(handle_config_error)
+            d.addBoth(self._call_hook, 'running')
+            # we've handled error via hook now (otherwise Twisted will
+            # want to fail some things)
+            d.addErrback(lambda _: None)
+            return d
 
         from twisted.internet import reactor
-        reactor.callWhenRunning(start)
+        x = reactor.callWhenRunning(start)
+        print("DING {}".format(x))
 
 
 class DaemonizeTahoeNodePlugin(object):

src/allmydata/test/cli/test_daemonize.py

@@ -4,6 +4,7 @@ from mock import patch, Mock
 from StringIO import StringIO
 from sys import getfilesystemencoding
 from twisted.trial import unittest
+from twisted.internet import defer
 from allmydata.scripts import runner
 from allmydata.scripts.tahoe_daemonize import identify_node_type
 from allmydata.scripts.tahoe_daemonize import DaemonizeTahoeNodePlugin
@@ -45,23 +46,32 @@ class Util(unittest.TestCase):
 
         self.assertTrue(service is not None)
 
+#    @defer.inlineCallbacks
     def test_daemonize_no_keygen(self):
         tmpdir = self.mktemp()
         plug = DaemonizeTahoeNodePlugin('key-generator', tmpdir)
 
-        with patch('twisted.internet.reactor') as r:
+        if True:#with patch('twisted.internet.reactor') as r:
             def call(fn, *args, **kw):
                 fn()
-            r.callWhenRunning = call
+#            r.callWhenRunning = call
-            r.stop = lambda: None
+#            r.stop = 'foo'
             service = plug.makeService(None)
             service.parent = Mock()
-            with self.assertRaises(ValueError) as ctx:
+            # we'll raise ValueError because there's no key-generator
+            # .. BUT we do this in an async function called via
+            # "callWhenRunning" .. hence using a hook
+            d = service.set_hook('running')
             service.startService()
+            def done(f):
+                print("DONE {}".format(f))
                 self.assertIn(
                     "key-generator support removed",
-                str(ctx.exception)
+                    str(str(f)),#ctx.exception)
                 )
+                return None
+            d.addErrback(done)
+            return d
 
     def test_daemonize_unknown_nodetype(self):
         tmpdir = self.mktemp()

src/allmydata/test/no_network.py

@@ -31,6 +31,7 @@ from allmydata.client import create_client
 from allmydata.storage.server import StorageServer, storage_index_to_dir
 from allmydata.util import fileutil, idlib, hashutil
 from allmydata.util.hashutil import permute_server_hash
+from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.interfaces import IStorageBroker, IServer
 from .common import TEST_RSA_KEY_SIZE
 

src/allmydata/test/test_client.py

@@ -1,8 +1,9 @@
 import os, sys
+import mock
 import twisted
 from twisted.trial import unittest
 from twisted.application import service
-import mock
+from twisted.internet import defer
 
 import allmydata
 import allmydata.frontends.magic_folder
@@ -34,8 +35,9 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         os.mkdir(basedir)
         fileutil.write(os.path.join(basedir, "tahoe.cfg"), \
                            BASECONFIG)
-        client.create_client(basedir)
+        return client.create_client(basedir)
 
+    @defer.inlineCallbacks
     def test_comment(self):
         should_fail = [r"test#test", r"#testtest", r"test\\#test"]
         should_not_fail = [r"test\#test", r"test\\\#test", r"testtest"]
@@ -51,13 +53,14 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         for s in should_fail:
             self.failUnless(_Config._contains_unescaped_hash(s))
             write_config(s)
-            e = self.assertRaises(UnescapedHashError, client.create_client, basedir)
+            with self.assertRaises(UnescapedHashError) as ctx:
-            self.assertIn("[client]introducer.furl", str(e))
+                yield client.create_client(basedir)
+            self.assertIn("[client]introducer.furl", str(ctx.exception))
 
         for s in should_not_fail:
             self.failIf(_Config._contains_unescaped_hash(s))
             write_config(s)
-            client.create_client(basedir)
+            yield client.create_client(basedir)
 
     def test_unreadable_config(self):
         if sys.platform == "win32":
@@ -128,12 +131,13 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                        ("Found pre-Tahoe-LAFS-v1.3 configuration file" in str(m) and oldfile in str(m)) ]
             self.failIf(logged, (oldfile, logged_messages))
 
+    @defer.inlineCallbacks
     def test_secrets(self):
         basedir = "test_client.Basic.test_secrets"
         os.mkdir(basedir)
         fileutil.write(os.path.join(basedir, "tahoe.cfg"), \
                            BASECONFIG)
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         secret_fname = os.path.join(basedir, "private", "secret")
         self.failUnless(os.path.exists(secret_fname), secret_fname)
         renew_secret = c.get_renewal_secret()
@@ -141,22 +145,25 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         cancel_secret = c.get_cancel_secret()
         self.failUnless(base32.b2a(cancel_secret))
 
+    @defer.inlineCallbacks
     def test_nodekey_yes_storage(self):
         basedir = "test_client.Basic.test_nodekey_yes_storage"
         os.mkdir(basedir)
         fileutil.write(os.path.join(basedir, "tahoe.cfg"),
                        BASECONFIG)
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnless(c.get_long_nodeid().startswith("v0-"))
 
+    @defer.inlineCallbacks
     def test_nodekey_no_storage(self):
         basedir = "test_client.Basic.test_nodekey_no_storage"
         os.mkdir(basedir)
         fileutil.write(os.path.join(basedir, "tahoe.cfg"),
                        BASECONFIG + "[storage]\n" + "enabled = false\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnless(c.get_long_nodeid().startswith("v0-"))
 
+    @defer.inlineCallbacks
     def test_reserved_1(self):
         basedir = "client.Basic.test_reserved_1"
         os.mkdir(basedir)
@@ -165,9 +172,10 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "[storage]\n" + \
                            "enabled = true\n" + \
                            "reserved_space = 1000\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnlessEqual(c.getServiceNamed("storage").reserved_space, 1000)
 
+    @defer.inlineCallbacks
     def test_reserved_2(self):
         basedir = "client.Basic.test_reserved_2"
         os.mkdir(basedir)
@@ -176,9 +184,10 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "[storage]\n" + \
                            "enabled = true\n" + \
                            "reserved_space = 10K\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnlessEqual(c.getServiceNamed("storage").reserved_space, 10*1000)
 
+    @defer.inlineCallbacks
     def test_reserved_3(self):
         basedir = "client.Basic.test_reserved_3"
         os.mkdir(basedir)
@@ -187,10 +196,11 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "[storage]\n" + \
                            "enabled = true\n" + \
                            "reserved_space = 5mB\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnlessEqual(c.getServiceNamed("storage").reserved_space,
                              5*1000*1000)
 
+    @defer.inlineCallbacks
     def test_reserved_4(self):
         basedir = "client.Basic.test_reserved_4"
         os.mkdir(basedir)
@@ -199,10 +209,11 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "[storage]\n" + \
                            "enabled = true\n" + \
                            "reserved_space = 78Gb\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         self.failUnlessEqual(c.getServiceNamed("storage").reserved_space,
                              78*1000*1000*1000)
 
+    @defer.inlineCallbacks
     def test_reserved_bad(self):
         basedir = "client.Basic.test_reserved_bad"
         os.mkdir(basedir)
@@ -211,8 +222,10 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "[storage]\n" + \
                            "enabled = true\n" + \
                            "reserved_space = bogus\n")
-        self.failUnlessRaises(ValueError, client.create_client, basedir)
+        with self.assertRaises(ValueError) as ctx:
+            yield client.create_client(basedir)
 
+    @defer.inlineCallbacks
     def test_web_apiauthtoken(self):
         """
         Client loads the proper API auth token from disk
@@ -220,7 +233,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         basedir = u"client.Basic.test_web_apiauthtoken"
         create_node_dir(basedir, "testing")
 
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         # this must come after we create the client, as it will create
         # a new, random authtoken itself
         with open(os.path.join(basedir, "private", "api_auth_token"), "w") as f:
@@ -229,6 +242,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         token = c.get_auth_token()
         self.assertEqual("deadbeef", token)
 
+    @defer.inlineCallbacks
     def test_web_staticdir(self):
         basedir = u"client.Basic.test_web_staticdir"
         os.mkdir(basedir)
@@ -237,7 +251,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                        "[node]\n" +
                        "web.port = tcp:0:interface=127.0.0.1\n" +
                        "web.static = relative\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         w = c.getServiceNamed("webish")
         abs_basedir = fileutil.abspath_expanduser_unicode(basedir)
         expected = fileutil.abspath_expanduser_unicode(u"relative", abs_basedir)
@@ -245,6 +259,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
 
     # TODO: also test config options for SFTP.
 
+    @defer.inlineCallbacks
     def test_ftp_create(self):
         """
         configuration for sftpd results in it being started
@@ -260,9 +275,10 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                 'host_privkey_file = privkey\n'
             )
         with mock.patch('allmydata.frontends.sftpd.SFTPServer') as p:
-            client.create_client(basedir)
+            yield client.create_client(basedir)
         self.assertTrue(p.called)
 
+    @defer.inlineCallbacks
     def test_ftp_auth_keyfile(self):
         basedir = u"client.Basic.test_ftp_auth_keyfile"
         os.mkdir(basedir)
@@ -274,9 +290,10 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                         "accounts.file = private/accounts\n"))
         os.mkdir(os.path.join(basedir, "private"))
         fileutil.write(os.path.join(basedir, "private", "accounts"), "\n")
-        c = client.create_client(basedir) # just make sure it can be instantiated
+        c = yield client.create_client(basedir) # just make sure it can be instantiated
         del c
 
+    @defer.inlineCallbacks
     def test_ftp_auth_url(self):
         basedir = u"client.Basic.test_ftp_auth_url"
         os.mkdir(basedir)
@@ -286,7 +303,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                         "enabled = true\n"
                         "port = tcp:0:interface=127.0.0.1\n"
                         "accounts.url = http://0.0.0.0/\n"))
-        c = client.create_client(basedir) # just make sure it can be instantiated
+        c = yield client.create_client(basedir) # just make sure it can be instantiated
         del c
 
     def test_ftp_auth_no_accountfile_or_url(self):
@@ -297,7 +314,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                         "[ftpd]\n"
                         "enabled = true\n"
                         "port = tcp:0:interface=127.0.0.1\n"))
-        self.failUnlessRaises(NeedRootcapLookupScheme, client.create_client, basedir)
+        with self.assertRaises(NeedRootcapLookupScheme):
+            yield client.create_client(basedir)
 
     def _storage_dir_test(self, basedir, storage_path, expected_path):
         os.mkdir(basedir)
@@ -407,6 +425,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         sb.servers.clear()
         self.failUnlessReallyEqual(self._permute(sb, "one"), [])
 
+    @defer.inlineCallbacks
     def test_versions(self):
         basedir = "test_client.Basic.test_versions"
         os.mkdir(basedir)
@@ -414,7 +433,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            BASECONFIG + \
                            "[storage]\n" + \
                            "enabled = true\n")
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
         ss = c.getServiceNamed("storage")
         verdict = ss.remote_get_version()
         self.failUnlessReallyEqual(verdict["application-version"],
@@ -429,24 +448,27 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         self.failUnless("node.uptime" in stats)
         self.failUnless(isinstance(stats["node.uptime"], float))
 
+    @defer.inlineCallbacks
     def test_helper_furl(self):
         basedir = "test_client.Basic.test_helper_furl"
         os.mkdir(basedir)
 
+        @defer.inlineCallbacks
         def _check(config, expected_furl):
             fileutil.write(os.path.join(basedir, "tahoe.cfg"),
                            BASECONFIG + config)
-            c = client.create_client(basedir)
+            c = yield client.create_client(basedir)
             uploader = c.getServiceNamed("uploader")
             furl, connected = uploader.get_helper_info()
             self.failUnlessEqual(furl, expected_furl)
 
-        _check("", None)
+        yield _check("", None)
-        _check("helper.furl =\n", None)
+        yield _check("helper.furl =\n", None)
-        _check("helper.furl = \n", None)
+        yield _check("helper.furl = \n", None)
-        _check("helper.furl = None", None)
+        yield _check("helper.furl = None", None)
-        _check("helper.furl = pb://blah\n", "pb://blah")
+        yield _check("helper.furl = pb://blah\n", "pb://blah")
 
+    @defer.inlineCallbacks
     def test_create_magic_folder_service(self):
         boom = False
         class Boom(Exception):
@@ -496,7 +518,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         # which config-entry should be missing?
         fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
                        config + "local.directory = " + local_dir_utf8 + "\n")
-        self.failUnlessRaises(IOError, client.create_client, basedir1)
+        with self.assertRaises(IOError):
+            yield client.create_client(basedir1)
 
         # local.directory entry missing .. but that won't be an error
         # now, it'll just assume there are not magic folders
@@ -508,11 +531,13 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
 
         fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
                        config.replace("[magic_folder]\n", "[drop_upload]\n"))
-        self.failUnlessRaises(OldConfigOptionError, client.create_client, basedir1)
+
+        with self.assertRaises(OldConfigOptionError):
+            yield client.create_client(basedir1)
 
         fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
                        config + "local.directory = " + local_dir_utf8 + "\n")
-        c1 = client.create_client(basedir1)
+        c1 = yield client.create_client(basedir1)
         magicfolder = c1.getServiceNamed('magic-folder')
         self.failUnless(isinstance(magicfolder, MockMagicFolder), magicfolder)
         self.failUnlessReallyEqual(magicfolder.client, c1)
@@ -534,7 +559,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                        "local.directory = " + local_dir_utf8 + "\n")
         fileutil.write(os.path.join(basedir2, "private", "magic_folder_dircap"), "URI:DIR2:blah")
         fileutil.write(os.path.join(basedir2, "private", "collective_dircap"), "URI:DIR2:meow")
-        self.failUnlessRaises(Boom, client.create_client, basedir2)
+        with self.assertRaises(Boom):
+            yield client.create_client(basedir2)
 
 
 def flush_but_dont_ignore(res):
@@ -554,33 +580,35 @@ class Run(unittest.TestCase, testutil.StallMixin):
         d.addBoth(flush_but_dont_ignore)
         return d
 
+    @defer.inlineCallbacks
     def test_loadable(self):
         basedir = "test_client.Run.test_loadable"
         os.mkdir(basedir)
         dummy = "pb://wl74cyahejagspqgy4x5ukrvfnevlknt@127.0.0.1:58889/bogus"
         fileutil.write(os.path.join(basedir, "tahoe.cfg"), BASECONFIG_I % dummy)
         fileutil.write(os.path.join(basedir, client._Client.EXIT_TRIGGER_FILE), "")
-        client.create_client(basedir)
+        yield client.create_client(basedir)
 
+    @defer.inlineCallbacks
     def test_reloadable(self):
         basedir = "test_client.Run.test_reloadable"
         os.mkdir(basedir)
         dummy = "pb://wl74cyahejagspqgy4x5ukrvfnevlknt@127.0.0.1:58889/bogus"
         fileutil.write(os.path.join(basedir, "tahoe.cfg"), BASECONFIG_I % dummy)
-        c1 = client.create_client(basedir)
+        c1 = yield client.create_client(basedir)
         c1.setServiceParent(self.sparent)
 
         # delay to let the service start up completely. I'm not entirely sure
         # this is necessary.
-        d = self.stall(delay=2.0)
+        yield self.stall(delay=2.0)
-        d.addCallback(lambda res: c1.disownServiceParent())
+        yield c1.disownServiceParent()
         # the cygwin buildslave seems to need more time to let the old
         # service completely shut down. When delay=0.1, I saw this test fail,
         # probably due to the logport trying to reclaim the old socket
         # number. This suggests that either we're dropping a Deferred
         # somewhere in the shutdown sequence, or that cygwin is just cranky.
-        d.addCallback(self.stall, delay=2.0)
+        yield self.stall(delay=2.0)
-        def _restart(res):
+
         # TODO: pause for slightly over one second, to let
         # Client._check_exit_trigger poll the file once. That will exercise
         # another few lines. Then add another test in which we don't
@@ -589,18 +617,18 @@ class Run(unittest.TestCase, testutil.StallMixin):
         # also change _check_exit_trigger to use it instead of a raw
         # reactor.stop, also instrument the shutdown event in an
         # attribute that we can check.)
-            c2 = client.create_client(basedir)
+        c2 = yield client.create_client(basedir)
         c2.setServiceParent(self.sparent)
-            return c2.disownServiceParent()
+        yield c2.disownServiceParent()
-        d.addCallback(_restart)
-        return d
 
 class NodeMaker(testutil.ReallyEqualMixin, unittest.TestCase):
+
+    @defer.inlineCallbacks
     def test_maker(self):
         basedir = "client/NodeMaker/maker"
         fileutil.make_dirs(basedir)
         fileutil.write(os.path.join(basedir, "tahoe.cfg"), BASECONFIG)
-        c = client.create_client(basedir)
+        c = yield client.create_client(basedir)
 
         n = c.create_node_from_uri("URI:CHK:6nmrpsubgbe57udnexlkiwzmlu:bjt7j6hshrlmadjyr7otq3dc24end5meo5xcr5xe5r663po6itmq:3:10:7277")
         self.failUnless(IFilesystemNode.providedBy(n))