From 488a04cb9b8af59171f02908fb4d6d00474865c9 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 17:42:06 -0600
Subject: [PATCH 01/22] exit when stdin closes

---
 src/allmydata/scripts/tahoe_run.py | 47 ++++++++++++++++++++++++++--
 src/allmydata/test/test_runner.py  | 49 ++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py
index 51be32ee3..68578a2a1 100644
--- a/src/allmydata/scripts/tahoe_run.py
+++ b/src/allmydata/scripts/tahoe_run.py
@@ -20,7 +20,9 @@ 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
+from twisted.internet.defer import maybeDeferred, Deferred
+from twisted.internet.protocol import Protocol
+from twisted.internet.stdio import StandardIO
 from twisted.application.service import Service
 
 from allmydata.scripts.default_nodedir import _default_nodedir
@@ -148,6 +150,8 @@ class DaemonizeTheRealService(Service, HookMixin):
 
     def startService(self):
 
+        from twisted.internet import reactor
+
         def start():
             node_to_instance = {
                 u"client": lambda: maybeDeferred(namedAny("allmydata.client.create_client"), self.basedir),
@@ -187,12 +191,14 @@ class DaemonizeTheRealService(Service, HookMixin):
 
             def created(srv):
                 srv.setServiceParent(self.parent)
+                # exiting on stdin-closed facilitates cleanup when run
+                # as a subprocess
+                on_stdin_close(reactor, reactor.stop)
             d.addCallback(created)
             d.addErrback(handle_config_error)
             d.addBoth(self._call_hook, 'running')
             return d
 
-        from twisted.internet import reactor
         reactor.callWhenRunning(start)
 
 
@@ -206,6 +212,43 @@ class DaemonizeTahoeNodePlugin(object):
         return DaemonizeTheRealService(self.nodetype, self.basedir, so)
 
 
+def on_stdin_close(reactor, fn):
+    """
+    Arrange for the function `fn` to run when our stdin closes
+    """
+    when_closed_d = Deferred()
+
+    class WhenClosed(Protocol):
+        """
+        Notify a Deferred when our connection is lost .. as this is passed
+        to twisted's StandardIO class, it is used to detect our parent
+        going away.
+        """
+
+        def connectionLost(self, reason):
+            when_closed_d.callback(None)
+
+    def on_close(arg):
+        try:
+            fn()
+        except Exception:
+            # for our "exit" use-case, this will _mostly_ just be
+            # ReactorNotRunning (because we're already shutting down
+            # when our stdin closes) but no matter what "bad thing"
+            # happens we just want to ignore it.
+            pass
+        return arg
+
+    when_closed_d.addBoth(on_close)
+    # we don't need to do anything with this instance because it gets
+    # hooked into the reactor and thus remembered
+    StandardIO(
+        proto=WhenClosed(),
+        reactor=reactor,
+    )
+    return None
+
+
 def run(config, runApp=twistd.runApp):
     """
     Runs a Tahoe-LAFS node in the foreground.
diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index 3eb6b8a34..fdd31c37d 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -46,6 +46,9 @@ from twisted.internet.defer import (
     inlineCallbacks,
     DeferredList,
 )
+from twisted.internet.testing import (
+    MemoryReactorClock,
+)
 from twisted.python.filepath import FilePath
 from twisted.python.runtime import (
     platform,
@@ -57,6 +60,9 @@ import allmydata
 from allmydata.scripts.runner import (
     parse_options,
 )
+from allmydata.scripts.tahoe_run import (
+    on_stdin_close,
+)
 
 from .common import (
     PIPE,
@@ -621,3 +627,46 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin):
         # What's left is a perfect indicator that the process has exited and
         # we won't get blamed for leaving the reactor dirty.
         yield client_running
+
+
+class OnStdinCloseTests(SyncTestCase):
+    """
+    Tests for on_stdin_close
+    """
+
+    def test_close_called(self):
+        """
+        our on-close method is called when stdin closes
+        """
+        reactor = MemoryReactorClock()
+        called = []
+
+        def onclose():
+            called.append(True)
+        on_stdin_close(reactor, onclose)
+        self.assertEqual(called, [])
+
+        reader = list(reactor.readers)[0]
+        reader.loseConnection()
+        reactor.advance(1)  # ProcessReader does a callLater(0, ..)
+
+        self.assertEqual(called, [True])
+
+    def test_exception_ignored(self):
+        """
+        an exception from or on-close function is ignored
+        """
+        reactor = MemoryReactorClock()
+        called = []
+
+        def onclose():
+            called.append(True)
+            raise RuntimeError("unexpected error")
+        on_stdin_close(reactor, onclose)
+        self.assertEqual(called, [])
+
+        reader = list(reactor.readers)[0]
+        reader.loseConnection()
+        reactor.advance(1)  # ProcessReader does a callLater(0, ..)
+
+        self.assertEqual(called, [True])

From 1e6381ca7f45bea90dc3518abd7ad0c0f79d7670 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 17:44:01 -0600
Subject: [PATCH 02/22] news

---
 newsfragments/3921.feature | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 newsfragments/3921.feature

diff --git a/newsfragments/3921.feature b/newsfragments/3921.feature
new file mode 100644
index 000000000..f2c3a98bd
--- /dev/null
+++ b/newsfragments/3921.feature
@@ -0,0 +1,5 @@
+Automatically exit when stdin is closed
+
+This facilitates subprocess management, specifically cleanup.
+When a parent process is running tahoe and exits without time to do "proper" cleanup at least the stdin descriptor will be closed.
+Subsequently "tahoe run" notices this and exits.
\ No newline at end of file

From 768829e993d957e6d4a78134fcd16cb5d2e92295 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 21:22:45 -0600
Subject: [PATCH 03/22] more robust

---
 src/allmydata/test/test_runner.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index fdd31c37d..8424bec6a 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -646,7 +646,8 @@ class OnStdinCloseTests(SyncTestCase):
         on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        reader = list(reactor.readers)[0]
+        for reader in reactor.getReaders():
+            reader.loseConnection()
         reader.loseConnection()
         reactor.advance(1)  # ProcessReader does a callLater(0, ..)
 
@@ -665,7 +666,8 @@ class OnStdinCloseTests(SyncTestCase):
         on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        reader = list(reactor.readers)[0]
+        for reader in reactor.getReaders():
+            reader.loseConnection()
         reader.loseConnection()
         reactor.advance(1)  # ProcessReader does a callLater(0, ..)
 

From 00c785ec7697167cbe8c37b9f50bd9be95690395 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 21:47:28 -0600
Subject: [PATCH 04/22] debug windows

---
 src/allmydata/test/test_runner.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index 8424bec6a..74d7ac59f 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -646,6 +646,8 @@ class OnStdinCloseTests(SyncTestCase):
         on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
+        print("READERS", reactor.getReaders())
+
         for reader in reactor.getReaders():
             reader.loseConnection()
         reader.loseConnection()

From decb36a8f6f4d755b51c6f2b2e624d77dcf31899 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 22:20:07 -0600
Subject: [PATCH 05/22] refactor for Windows testing

---
 src/allmydata/scripts/tahoe_run.py |  6 +++---
 src/allmydata/test/test_runner.py  | 27 ++++++++++++++++-----------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py
index 68578a2a1..63dc351b1 100644
--- a/src/allmydata/scripts/tahoe_run.py
+++ b/src/allmydata/scripts/tahoe_run.py
@@ -241,12 +241,12 @@ def on_stdin_close(reactor, fn):
 
     when_closed_d.addBoth(on_close)
     # we don't need to do anything with this instance because it gets
-    # hooked into the reactor and thus remembered
-    StandardIO(
+    # hooked into the reactor and thus remembered .. but we return it
+    # for Windows testing purposes.
+    return StandardIO(
         proto=WhenClosed(),
         reactor=reactor,
     )
-    return None
 
 
 def run(config, runApp=twistd.runApp):
diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index 74d7ac59f..fc3ed9618 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -643,15 +643,18 @@ class OnStdinCloseTests(SyncTestCase):
 
         def onclose():
             called.append(True)
-        on_stdin_close(reactor, onclose)
+        proto = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        print("READERS", reactor.getReaders())
-
-        for reader in reactor.getReaders():
-            reader.loseConnection()
-        reader.loseConnection()
-        reactor.advance(1)  # ProcessReader does a callLater(0, ..)
+        # one Unix we can just close all the readers, correctly
+        # "simulating" a stdin close .. of course, Windows has to be
+        # difficult
+        if platform.isWindows():
+            proto.loseConnection()
+        else:
+            for reader in reactor.getReaders():
+                reader.loseConnection()
+            reactor.advance(1)  # ProcessReader does a callLater(0, ..)
 
         self.assertEqual(called, [True])
 
@@ -668,9 +671,11 @@ class OnStdinCloseTests(SyncTestCase):
         on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        for reader in reactor.getReaders():
-            reader.loseConnection()
-        reader.loseConnection()
-        reactor.advance(1)  # ProcessReader does a callLater(0, ..)
+        if platform.isWindows():
+            proto.loseConnection()
+        else:
+            for reader in reactor.getReaders():
+                reader.loseConnection()
+            reactor.advance(1)  # ProcessReader does a callLater(0, ..)
 
         self.assertEqual(called, [True])

From 711f6d39e7281cad447fd51ae06c16e8d3247384 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Sep 2022 22:29:19 -0600
Subject: [PATCH 06/22] missing proto

---
 src/allmydata/test/test_runner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index fc3ed9618..c4bdee3fb 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -668,7 +668,7 @@ class OnStdinCloseTests(SyncTestCase):
         def onclose():
             called.append(True)
             raise RuntimeError("unexpected error")
-        on_stdin_close(reactor, onclose)
+        proto = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
         if platform.isWindows():

From 1058e50c50f47635646c13d20b264d1579cf3de4 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 8 Sep 2022 16:30:30 -0600
Subject: [PATCH 07/22] close properly

---
 src/allmydata/test/test_runner.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index c4bdee3fb..bce5b3c20 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -646,11 +646,12 @@ class OnStdinCloseTests(SyncTestCase):
         proto = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        # one Unix we can just close all the readers, correctly
+        # on Unix we can just close all the readers, correctly
         # "simulating" a stdin close .. of course, Windows has to be
         # difficult
         if platform.isWindows():
-            proto.loseConnection()
+            proto.writeConnectionLost()
+            proto.readConnectionLost()
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()
@@ -672,7 +673,8 @@ class OnStdinCloseTests(SyncTestCase):
         self.assertEqual(called, [])
 
         if platform.isWindows():
-            proto.loseConnection()
+            proto.writeConnectionLost()
+            proto.readConnectionLost()
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()

From d7fe25f7c7ffc1e8eb9da45755085c5dd04c25d8 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Tue, 29 Nov 2022 10:49:20 -0500
Subject: [PATCH 08/22] Correct the assertion about how "not found" should be
 handled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Behavior verified visually against a live client node:

```
❯ curl -v 'http://localhost:3456/uri/URI:CHK:cmtcxq7hwxvfxan34yiev6ivhy:qvcekmjtoetdcw4kmi7b3rtblvgx7544crnwaqtiewemdliqsokq:1:1:1'
*   Trying 127.0.0.1:3456...
* Connected to localhost (127.0.0.1) port 3456 (#0)
> GET /uri/URI:CHK:cmtcxq7hwxvfxan34yiev6ivhy:qvcekmjtoetdcw4kmi7b3rtblvgx7544crnwaqtiewemdliqsokq:1:1:1 HTTP/1.1
> Host: localhost:3456
> User-Agent: curl/7.83.1
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 410 Gone
< X-Frame-Options: DENY
< Referrer-Policy: no-referrer
< Server: TwistedWeb/22.10.0
< Date: Tue, 29 Nov 2022 15:39:47 GMT
< Content-Type: text/plain;charset=utf-8
< Accept-Ranges: bytes
< Content-Length: 294
< ETag: ui2tnwl5lltj5clzpyff42jdce-
<
NoSharesError: no shares could be found. Zero shares usually indicates a corrupt URI, or that no servers were connected, but it might also indicate severe corruption. You should perform a filecheck on this object to learn more.

The full error message is:
* Connection #0 to host localhost left intact
no shares (need 1). Last failure: None
```
---
 src/allmydata/test/test_testing.py | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py
index 3715d1aca..07bebb7a1 100644
--- a/src/allmydata/test/test_testing.py
+++ b/src/allmydata/test/test_testing.py
@@ -9,18 +9,7 @@
 
 """
 Tests for the allmydata.testing helpers
-
-Ported to Python 3.
-
 """
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
-
-from future.utils import PY2
-if PY2:
-    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
 
 from twisted.internet.defer import (
     inlineCallbacks,
@@ -56,10 +45,12 @@ from testtools.matchers import (
     IsInstance,
     MatchesStructure,
     AfterPreprocessing,
+    Contains,
 )
 from testtools.twistedsupport import (
     succeeded,
 )
+from twisted.web.http import GONE
 
 
 class FakeWebTest(SyncTestCase):
@@ -144,7 +135,8 @@ class FakeWebTest(SyncTestCase):
 
     def test_download_missing(self):
         """
-        Error if we download a capability that doesn't exist
+        The response to a request to download a capability that doesn't exist
+        is 410 (GONE).
         """
 
         http_client = create_tahoe_treq_client()
@@ -157,7 +149,11 @@ class FakeWebTest(SyncTestCase):
             resp,
             succeeded(
                 MatchesStructure(
-                    code=Equals(500)
+                    code=Equals(GONE),
+                    content=AfterPreprocessing(
+                        lambda m: m(),
+                        succeeded(Contains(b"No data for")),
+                    ),
                 )
             )
         )

From 02aeb68f1731c58e146ccefd1d8ea99ad364b73d Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Tue, 29 Nov 2022 10:51:07 -0500
Subject: [PATCH 09/22] Take care with str vs bytes in the implementation

Also replace the intentional BAD_REQUEST with GONE for this case.
---
 src/allmydata/testing/web.py | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index bb858b555..6538dc3a4 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -6,22 +6,13 @@
 # This file is part of Tahoe-LAFS.
 #
 # See the docs/about.rst file for licensing information.
-"""Test-helpers for clients that use the WebUI.
-
-Ported to Python 3.
 
 """
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
-
-from future.utils import PY2
-if PY2:
-    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
-
+Test-helpers for clients that use the WebUI.
+"""
 
 import hashlib
+from typing import Dict
 
 import attr
 
@@ -147,7 +138,7 @@ class _FakeTahoeUriHandler(Resource, object):
 
     isLeaf = True
 
-    data = attr.ib(default=attr.Factory(dict))
+    data: Dict[bytes, bytes] = attr.ib(default=attr.Factory(dict))
     capability_generators = attr.ib(default=attr.Factory(dict))
 
     def _generate_capability(self, kind):
@@ -209,7 +200,7 @@ class _FakeTahoeUriHandler(Resource, object):
         capability = None
         for arg, value in uri.query:
             if arg == u"uri":
-                capability = value
+                capability = value.encode("utf-8")
         # it's legal to use the form "/uri/<capability>"
         if capability is None and request.postpath and request.postpath[0]:
             capability = request.postpath[0]
@@ -221,10 +212,9 @@ class _FakeTahoeUriHandler(Resource, object):
 
         # the user gave us a capability; if our Grid doesn't have any
         # data for it, that's an error.
-        capability = capability.encode('ascii')
         if capability not in self.data:
-            request.setResponseCode(http.BAD_REQUEST)
-            return u"No data for '{}'".format(capability.decode('ascii'))
+            request.setResponseCode(http.GONE)
+            return u"No data for '{}'".format(capability.decode('ascii')).encode("utf-8")
 
         return self.data[capability]
 

From 6c0e5f5807cf72d38b9fb4a9461d0fd085920360 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Tue, 29 Nov 2022 10:52:02 -0500
Subject: [PATCH 10/22] news fragment

---
 newsfragments/3874.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3874.minor

diff --git a/newsfragments/3874.minor b/newsfragments/3874.minor
new file mode 100644
index 000000000..e69de29bb

From 1eb4a4adf8ba6b6f3c9ac236753a020428e424c5 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 16:47:24 -0700
Subject: [PATCH 11/22] Update newsfragments/3921.feature

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>
---
 newsfragments/3921.feature | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newsfragments/3921.feature b/newsfragments/3921.feature
index f2c3a98bd..798aee817 100644
--- a/newsfragments/3921.feature
+++ b/newsfragments/3921.feature
@@ -1,4 +1,4 @@
-Automatically exit when stdin is closed
+`tahoe run ...` will now exit when its stdin is closed.
 
 This facilitates subprocess management, specifically cleanup.
 When a parent process is running tahoe and exits without time to do "proper" cleanup at least the stdin descriptor will be closed.

From 7ffcfcdb67213ef9002279f2a55b57a2789fa12d Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 16:47:40 -0700
Subject: [PATCH 12/22] Update src/allmydata/test/test_runner.py

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>
---
 src/allmydata/test/test_runner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index bce5b3c20..9830487f3 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -661,7 +661,7 @@ class OnStdinCloseTests(SyncTestCase):
 
     def test_exception_ignored(self):
         """
-        an exception from or on-close function is ignored
+        An exception from our on-close function is discarded.
         """
         reactor = MemoryReactorClock()
         called = []

From 3d43cbccc9ef24fb3168e89eb63a93d682584417 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 17:01:38 -0700
Subject: [PATCH 13/22] log less-specific failures

---
 src/allmydata/scripts/tahoe_run.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py
index 49507765e..aaf234b61 100644
--- a/src/allmydata/scripts/tahoe_run.py
+++ b/src/allmydata/scripts/tahoe_run.py
@@ -21,9 +21,11 @@ from twisted.scripts import twistd
 from twisted.python import usage
 from twisted.python.filepath import FilePath
 from twisted.python.reflect import namedAny
+from twisted.python.failure import Failure
 from twisted.internet.defer import maybeDeferred, Deferred
 from twisted.internet.protocol import Protocol
 from twisted.internet.stdio import StandardIO
+from twisted.internet.error import ReactorNotRunning
 from twisted.application.service import Service
 
 from allmydata.scripts.default_nodedir import _default_nodedir
@@ -238,12 +240,15 @@ def on_stdin_close(reactor, fn):
     def on_close(arg):
         try:
             fn()
+        except ReactorNotRunning:
+            pass
         except Exception:
-            # for our "exit" use-case, this will _mostly_ just be
+            # for our "exit" use-case failures will _mostly_ just be
             # ReactorNotRunning (because we're already shutting down
             # when our stdin closes) but no matter what "bad thing"
-            # happens we just want to ignore it.
-            pass
+            # happens we just want to ignore it .. although other
+            # errors might be interesting so we'll log those
+            print(Failure())
         return arg
 
     when_closed_d.addBoth(on_close)

From 36ed554627dc393172d8532b6a66c1bcdbb22334 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 17:03:48 -0700
Subject: [PATCH 14/22] proto -> transport

---
 src/allmydata/test/test_runner.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index 52ec84ae6..cd09e5aa0 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -644,15 +644,15 @@ class OnStdinCloseTests(SyncTestCase):
 
         def onclose():
             called.append(True)
-        proto = on_stdin_close(reactor, onclose)
+        transport = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
         # on Unix we can just close all the readers, correctly
         # "simulating" a stdin close .. of course, Windows has to be
         # difficult
         if platform.isWindows():
-            proto.writeConnectionLost()
-            proto.readConnectionLost()
+            transport.writeConnectionLost()
+            transport.readConnectionLost()
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()
@@ -670,12 +670,12 @@ class OnStdinCloseTests(SyncTestCase):
         def onclose():
             called.append(True)
             raise RuntimeError("unexpected error")
-        proto = on_stdin_close(reactor, onclose)
+        transport = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
         if platform.isWindows():
-            proto.writeConnectionLost()
-            proto.readConnectionLost()
+            transport.writeConnectionLost()
+            transport.readConnectionLost()
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()

From 20b3594d128e4a90b56ef516a713d44156d17122 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 17:05:58 -0700
Subject: [PATCH 15/22] exarkun wants a helper

---
 src/allmydata/test/test_runner.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py
index cd09e5aa0..a84fa28f8 100644
--- a/src/allmydata/test/test_runner.py
+++ b/src/allmydata/test/test_runner.py
@@ -630,6 +630,15 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin):
         yield client_running
 
 
+def _simulate_windows_stdin_close(stdio):
+    """
+    on Unix we can just close all the readers, correctly "simulating"
+    a stdin close .. of course, Windows has to be difficult
+    """
+    stdio.writeConnectionLost()
+    stdio.readConnectionLost()
+
+
 class OnStdinCloseTests(SyncTestCase):
     """
     Tests for on_stdin_close
@@ -647,12 +656,8 @@ class OnStdinCloseTests(SyncTestCase):
         transport = on_stdin_close(reactor, onclose)
         self.assertEqual(called, [])
 
-        # on Unix we can just close all the readers, correctly
-        # "simulating" a stdin close .. of course, Windows has to be
-        # difficult
         if platform.isWindows():
-            transport.writeConnectionLost()
-            transport.readConnectionLost()
+            _simulate_windows_stdin_close(transport)
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()
@@ -674,8 +679,7 @@ class OnStdinCloseTests(SyncTestCase):
         self.assertEqual(called, [])
 
         if platform.isWindows():
-            transport.writeConnectionLost()
-            transport.readConnectionLost()
+            _simulate_windows_stdin_close(transport)
         else:
             for reader in reactor.getReaders():
                 reader.loseConnection()

From 89b6a008d2277e17832acf3afb16c6e71f88715c Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 23:24:24 -0700
Subject: [PATCH 16/22] since 'coverage report' is what fails with disk-space
 on windows, try turning it off

---
 tox.ini | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tox.ini b/tox.ini
index fc95a0469..9b6dc8756 100644
--- a/tox.ini
+++ b/tox.ini
@@ -86,7 +86,7 @@ commands =
          coverage: python -b -m coverage run -m twisted.trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors --reporter=timing} {posargs:{env:TEST_SUITE}}
          coverage: coverage combine
          coverage: coverage xml
-         coverage: coverage report
+##         coverage: coverage report
 
 [testenv:integration]
 basepython = python3
@@ -99,7 +99,7 @@ commands =
          # NOTE: 'run with "py.test --keep-tempdir -s -v integration/" to debug failures'
          py.test --timeout=1800 --coverage -s -v {posargs:integration}
          coverage combine
-         coverage report
+##         coverage report
 
 
 [testenv:codechecks]

From 3d831f653ba20286ea94e512cf0e87882cbc9e26 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Dec 2022 23:58:53 -0700
Subject: [PATCH 17/22] cleanup

---
 .github/workflows/ci.yml | 2 +-
 tox.ini                  | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 163266613..15e7d8fa4 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -63,7 +63,7 @@ jobs:
             python-version: "pypy-3.7"
           - os: ubuntu-latest
             python-version: "pypy-3.8"
-    
+
     steps:
       # See https://github.com/actions/checkout. A fetch-depth of 0
       # fetches all tags and branches.
diff --git a/tox.ini b/tox.ini
index 9b6dc8756..db4748033 100644
--- a/tox.ini
+++ b/tox.ini
@@ -86,7 +86,6 @@ commands =
          coverage: python -b -m coverage run -m twisted.trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors --reporter=timing} {posargs:{env:TEST_SUITE}}
          coverage: coverage combine
          coverage: coverage xml
-##         coverage: coverage report
 
 [testenv:integration]
 basepython = python3
@@ -99,7 +98,6 @@ commands =
          # NOTE: 'run with "py.test --keep-tempdir -s -v integration/" to debug failures'
          py.test --timeout=1800 --coverage -s -v {posargs:integration}
          coverage combine
-##         coverage report
 
 
 [testenv:codechecks]

From 9619e286f4e0f50c20975f5789418eed09f4e350 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 2 Dec 2022 08:16:02 -0500
Subject: [PATCH 18/22] Switch the web testing double to BytesKeyDict

This will catch more str/bytes errors by default than `dict`
---
 src/allmydata/testing/web.py   |  3 ++-
 src/allmydata/util/dictutil.py | 34 +++++++---------------------------
 2 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index 6538dc3a4..a687e5480 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -45,6 +45,7 @@ import allmydata.uri
 from allmydata.util import (
     base32,
 )
+from ..util.dictutil import BytesKeyDict
 
 
 __all__ = (
@@ -138,7 +139,7 @@ class _FakeTahoeUriHandler(Resource, object):
 
     isLeaf = True
 
-    data: Dict[bytes, bytes] = attr.ib(default=attr.Factory(dict))
+    data: BytesKeyDict[bytes, bytes] = attr.ib(default=attr.Factory(BytesKeyDict))
     capability_generators = attr.ib(default=attr.Factory(dict))
 
     def _generate_capability(self, kind):
diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py
index 5971d26f6..0a7df0a38 100644
--- a/src/allmydata/util/dictutil.py
+++ b/src/allmydata/util/dictutil.py
@@ -1,21 +1,6 @@
 """
 Tools to mess with dicts.
-
-Ported to Python 3.
 """
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
-
-from future.utils import PY2
-if PY2:
-    # IMPORTANT: We deliberately don't import dict. The issue is that we're
-    # subclassing dict, so we'd end up exposing Python 3 dict APIs to lots of
-    # code that doesn't support it.
-    from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, list, object, range, str, max, min  # noqa: F401
-from six import ensure_str
-
 
 class DictOfSets(dict):
     def add(self, key, value):
@@ -104,7 +89,7 @@ def _make_enforcing_override(K, method_name):
             raise TypeError("{} must be of type {}".format(
                 repr(key), self.KEY_TYPE))
         return getattr(dict, method_name)(self, key, *args, **kwargs)
-    f.__name__ = ensure_str(method_name)
+    f.__name__ = method_name
     setattr(K, method_name, f)
 
 for _method_name in ["__setitem__", "__getitem__", "setdefault", "get",
@@ -113,18 +98,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get",
 del _method_name
 
 
-if PY2:
-    # No need for enforcement, can use either bytes or unicode as keys and it's
-    # fine.
-    BytesKeyDict = UnicodeKeyDict = dict
-else:
-    class BytesKeyDict(_TypedKeyDict):
-        """Keys should be bytes."""
+class BytesKeyDict(_TypedKeyDict):
+    """Keys should be bytes."""
 
-        KEY_TYPE = bytes
+    KEY_TYPE = bytes
 
 
-    class UnicodeKeyDict(_TypedKeyDict):
-        """Keys should be unicode strings."""
+class UnicodeKeyDict(_TypedKeyDict):
+    """Keys should be unicode strings."""
 
-        KEY_TYPE = str
+    KEY_TYPE = str

From a84b278ecdaf3263030ad9817961d70cdfdfd741 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 2 Dec 2022 08:26:15 -0500
Subject: [PATCH 19/22] support older pythons

---
 src/allmydata/testing/web.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index a687e5480..be7878d57 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -11,6 +11,8 @@
 Test-helpers for clients that use the WebUI.
 """
 
+from __future__ import annotations
+
 import hashlib
 from typing import Dict
 

From b40d882fceb20fa102ca32540819555a08b2fdf1 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 2 Dec 2022 08:28:22 -0500
Subject: [PATCH 20/22] remove unused import

---
 src/allmydata/testing/web.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index be7878d57..4af2603a8 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -14,7 +14,6 @@ Test-helpers for clients that use the WebUI.
 from __future__ import annotations
 
 import hashlib
-from typing import Dict
 
 import attr
 

From c6cc3708f45c9e581a719852d4f1082528941701 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 2 Dec 2022 08:38:46 -0500
Subject: [PATCH 21/22] Fixup the annotations a bit

---
 src/allmydata/testing/web.py   |  2 +-
 src/allmydata/util/dictutil.py | 15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index 4af2603a8..72ecd7161 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -140,7 +140,7 @@ class _FakeTahoeUriHandler(Resource, object):
 
     isLeaf = True
 
-    data: BytesKeyDict[bytes, bytes] = attr.ib(default=attr.Factory(BytesKeyDict))
+    data: BytesKeyDict[bytes] = attr.ib(default=attr.Factory(BytesKeyDict))
     capability_generators = attr.ib(default=attr.Factory(dict))
 
     def _generate_capability(self, kind):
diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py
index 0a7df0a38..c436ab963 100644
--- a/src/allmydata/util/dictutil.py
+++ b/src/allmydata/util/dictutil.py
@@ -2,6 +2,10 @@
 Tools to mess with dicts.
 """
 
+from __future__ import annotations
+
+from typing import TypeVar, Type
+
 class DictOfSets(dict):
     def add(self, key, value):
         if key in self:
@@ -64,7 +68,10 @@ class AuxValueDict(dict):
         self.auxilliary[key] = auxilliary
 
 
-class _TypedKeyDict(dict):
+K = TypeVar("K")
+V = TypeVar("V")
+
+class _TypedKeyDict(dict[K, V]):
     """Dictionary that enforces key type.
 
     Doesn't override everything, but probably good enough to catch most
@@ -73,7 +80,7 @@ class _TypedKeyDict(dict):
     Subclass and override KEY_TYPE.
     """
 
-    KEY_TYPE = object
+    KEY_TYPE: Type[K]
 
     def __init__(self, *args, **kwargs):
         dict.__init__(self, *args, **kwargs)
@@ -98,13 +105,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get",
 del _method_name
 
 
-class BytesKeyDict(_TypedKeyDict):
+class BytesKeyDict(_TypedKeyDict[bytes, V]):
     """Keys should be bytes."""
 
     KEY_TYPE = bytes
 
 
-class UnicodeKeyDict(_TypedKeyDict):
+class UnicodeKeyDict(_TypedKeyDict[str, V]):
     """Keys should be unicode strings."""
 
     KEY_TYPE = str

From c542b8463707cd5720822de260f79b61c7582994 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Fri, 2 Dec 2022 08:47:07 -0500
Subject: [PATCH 22/22] remove the annotations

everything is broken on older pythons
---
 src/allmydata/testing/web.py   |  2 +-
 src/allmydata/util/dictutil.py | 15 ++++-----------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py
index 72ecd7161..4f68b3774 100644
--- a/src/allmydata/testing/web.py
+++ b/src/allmydata/testing/web.py
@@ -140,7 +140,7 @@ class _FakeTahoeUriHandler(Resource, object):
 
     isLeaf = True
 
-    data: BytesKeyDict[bytes] = attr.ib(default=attr.Factory(BytesKeyDict))
+    data: BytesKeyDict = attr.ib(default=attr.Factory(BytesKeyDict))
     capability_generators = attr.ib(default=attr.Factory(dict))
 
     def _generate_capability(self, kind):
diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py
index c436ab963..0a7df0a38 100644
--- a/src/allmydata/util/dictutil.py
+++ b/src/allmydata/util/dictutil.py
@@ -2,10 +2,6 @@
 Tools to mess with dicts.
 """
 
-from __future__ import annotations
-
-from typing import TypeVar, Type
-
 class DictOfSets(dict):
     def add(self, key, value):
         if key in self:
@@ -68,10 +64,7 @@ class AuxValueDict(dict):
         self.auxilliary[key] = auxilliary
 
 
-K = TypeVar("K")
-V = TypeVar("V")
-
-class _TypedKeyDict(dict[K, V]):
+class _TypedKeyDict(dict):
     """Dictionary that enforces key type.
 
     Doesn't override everything, but probably good enough to catch most
@@ -80,7 +73,7 @@ class _TypedKeyDict(dict[K, V]):
     Subclass and override KEY_TYPE.
     """
 
-    KEY_TYPE: Type[K]
+    KEY_TYPE = object
 
     def __init__(self, *args, **kwargs):
         dict.__init__(self, *args, **kwargs)
@@ -105,13 +98,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get",
 del _method_name
 
 
-class BytesKeyDict(_TypedKeyDict[bytes, V]):
+class BytesKeyDict(_TypedKeyDict):
     """Keys should be bytes."""
 
     KEY_TYPE = bytes
 
 
-class UnicodeKeyDict(_TypedKeyDict[str, V]):
+class UnicodeKeyDict(_TypedKeyDict):
     """Keys should be unicode strings."""
 
     KEY_TYPE = str