From 1070466febc12665064d9f03ca8e61d37de447d0 Mon Sep 17 00:00:00 2001
From: Nathaniel Wesley Filardo
 <VP331RHQ115POU58JFRLKB7OPA0L18E3@cmx.ietfng.org>
Date: Sun, 17 Feb 2019 18:32:16 +0000
Subject: [PATCH] Revise fifo{,sock} (#2671)

Fixes to #2650:

- Convert fifosock to returning tables containing ctors
- Improve docs
- Add a missed :on("sent", nil) in the http server
---
 docs/lua-modules/fifosock.md      | 28 ++++++++++++++++++++--------
 lua_examples/telnet/telnet.lua    |  2 +-
 lua_modules/fifo/fifosock.lua     |  8 ++++----
 lua_modules/fifo/fifosocktest.lua |  2 +-
 lua_modules/http/httpserver.lua   |  3 ++-
 5 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/docs/lua-modules/fifosock.md b/docs/lua-modules/fifosock.md
index 38d69dbd..e12dcfae 100644
--- a/docs/lua-modules/fifosock.md
+++ b/docs/lua-modules/fifosock.md
@@ -3,23 +3,35 @@
 | :----- | :-------------------- | :---------- | :------ |
 | 2019-02-10 | [TerryE](https://github.com/TerryE) | [nwf](https://github.com/nwf) | [fifosock.lua](../../lua_modules/fifo/fifosock.lua) |
 
-This module provides a convenient, efficient wrapper around the `net.socket`
-`send` method.  It ensures in-order transmission while striving to minimize
-memory footprint and packet count by coalescing queued strings.  It also serves
-as a detailed, worked example of the `fifo` module.
+This module provides a moderately convenient, efficient wrapper around the
+`net.socket` `send` method.  It ensures in-order transmission while striving to
+minimize memory footprint and packet count by coalescing queued strings.  It
+also serves as a detailed, worked example of the `fifo` module.
 
 ## Use
 ```lua
-ssend = (require "fifosock")(sock)
-
+ssend = (require "fifosock").wrap(sock)
 ssend("hello, ") ssend("world\n")
+
+-- when finished
+ssend = nil
+sock:on("sent", nil)
 ```
 
 Once the `sock`et has been wrapped, one should use only the resulting `ssend`
 function in lieu of `sock:send`, and one should not change the
-`sock:on("sent")` callback.
+`sock:on("sent")` callback for the duration of the connection.
 
-### Advanced Use
+Use of this module creates a circular reference through the Lua registry: the
+socket points at the fifosock wrapper, which points back at the socket.  As
+such, it is vitally important to break this cycle when the socket has outlived
+its use.  **The usual garbage collection will not be able to reclaim abandoned
+wrapped sockets**.  The user of `fifosock` must, when disposing of the socket,
+unwire the wrapper, by calling `sock:on("sent", nil)` and should drop all
+references to `ssend`; a convenient place to do this is in the
+`sock:on("disconnect")` callback.
+
+## Advanced Use
 
 In addition to passing strings representing part of the stream to be sent, it
 is possible to pass the resulting `ssend` function *functions*.  These
diff --git a/lua_examples/telnet/telnet.lua b/lua_examples/telnet/telnet.lua
index bddb9c38..328d5cbc 100644
--- a/lua_examples/telnet/telnet.lua
+++ b/lua_examples/telnet/telnet.lua
@@ -31,7 +31,7 @@ local node, table, tmr, wifi, uwrite,     tostring =
       node, table, tmr, wifi, uart.write, tostring
 
 local function telnet_listener(socket) 
-  local queueLine = (require "fifosock")(socket)
+  local queueLine = (require "fifosock").wrap(socket)
 
   local function receiveLine(s, line)
     node.input(line)
diff --git a/lua_modules/fifo/fifosock.lua b/lua_modules/fifo/fifosock.lua
index 7a149a1c..c453fb13 100644
--- a/lua_modules/fifo/fifosock.lua
+++ b/lua_modules/fifo/fifosock.lua
@@ -15,11 +15,9 @@ local concat = table.concat
 local insert = table.insert
 local gc     = collectgarbage
 
-local fifo = require "fifo"
-
-return function(sock)
+local function wrap(sock)
   -- the two fifos
-  local fsmall, lsmall, fbig = {}, 0, fifo.new()
+  local fsmall, lsmall, fbig = {}, 0, (require "fifo").new()
 
   -- ssend last aggregation string and aggregate count
   local ssla, sslan = nil, 0
@@ -132,3 +130,5 @@ return function(sock)
     end
   end
 end
+
+return { wrap = wrap }
diff --git a/lua_modules/fifo/fifosocktest.lua b/lua_modules/fifo/fifosocktest.lua
index 54aeb8d6..b2424c46 100644
--- a/lua_modules/fifo/fifosocktest.lua
+++ b/lua_modules/fifo/fifosocktest.lua
@@ -33,7 +33,7 @@ local fakesock = {
 local function sent() vprint("SENT") fakesock.cb() end
 
 -- And wrap a fifosock around this fake socket
-local fsend = require "fifosock" (fakesock)
+local fsend = (require "fifosock").wrap(fakesock)
 
 -- Verify that the next unconsumed output is as indicated
 local function fcheck(x)
diff --git a/lua_modules/http/httpserver.lua b/lua_modules/http/httpserver.lua
index 8455c688..ba269603 100644
--- a/lua_modules/http/httpserver.lua
+++ b/lua_modules/http/httpserver.lua
@@ -79,7 +79,7 @@ do
   ------------------------------------------------------------------------------
   local http_handler = function(handler)
     return function(conn)
-      local csend = (require "fifosock")(conn)
+      local csend = (require "fifosock").wrap(conn)
       local cfini = function()
         conn:on("receive", nil)
         conn:on("disconnection", nil)
@@ -89,6 +89,7 @@ do
       local buf = ""
       local method, url
       local ondisconnect = function(conn)
+	conn.on("sent", nil)
         collectgarbage("collect")
       end
       -- header parser