bugfix for issue #26: Supervisor child_spec memory leaks (#29)

* Fixed child_spec memory leak.
* Excluded the name from the main supervisor, added one just to the dynamic supervisor.
* Made the supervisor name more module-like, so users can name their stacks after `ModuleNames` instead of just `:atoms`.
* Reincluded a `mix.lock` file.

Co-authored-by: Martin Chabot <chabotm@gmail.com>

Author: balena

.gitignore

@@ -28,6 +28,3 @@ erl_crash.dump
 
 # C/C++ NIF modules
 *.o
-
-# Ignore mix.lock
-mix.lock

config/dev.exs

@@ -1,4 +1,3 @@
 import Config
 
-config :logger, :console,
-  format: "$time $metadata[$level] $levelpad$message\n"
+config :logger, :console, format: "$time $metadata[$level] $levelpad$message\n"

lib/sippet.ex

@@ -193,21 +193,21 @@ defmodule Sippet do
 
   @doc false
   def start_link(options) when is_list(options) do
-    name =
-      case Keyword.fetch(options, :name) do
-        {:ok, name} when is_atom(name) ->
-          name
+    case Keyword.fetch(options, :name) do
+      {:ok, name} when is_atom(name) ->
+        :ok
 
-        {:ok, other} ->
-          raise ArgumentError, "expected :name to be an atom, got: #{inspect(other)}"
+      {:ok, other} ->
+        raise ArgumentError, "expected :name to be an atom, got: #{inspect(other)}"
 
-        :error ->
-          raise ArgumentError, "expected :name option to be present"
-      end
+      :error ->
+        raise ArgumentError, "expected :name option to be present"
+    end
 
-    Supervisor.start_link(__MODULE__, options, name: :"#{name}_sup")
+    Supervisor.start_link(__MODULE__, options)
   end
 
+  @doc false
   def child_spec(options) do
     %{
       id: __MODULE__,
@@ -218,9 +218,15 @@ defmodule Sippet do
   @impl true
   def init(options) do
     children = [
-      {Registry, [name: options[:name], keys: :unique, partitions: System.schedulers_online()]}
+      {Registry, name: options[:name], keys: :unique, partitions: System.schedulers_online()},
+      {DynamicSupervisor, strategy: :one_for_one, name: supervisor_name(options[:name])}
     ]
 
     Supervisor.init(children, strategy: :one_for_one)
   end
+
+  @doc false
+  def supervisor_name(name) do
+    Module.concat(name, "Supervisor")
+  end
 end

lib/sippet/router.ex

@@ -6,6 +6,8 @@ defmodule Sippet.Router do
 
   require Logger
 
+  import Sippet, only: [supervisor_name: 1]
+
   @doc false
   def handle_transport_message(sippet, iodata, from) when is_list(iodata) do
     binary =
@@ -63,6 +65,7 @@ defmodule Sippet.Router do
   defp ip_to_string(ip) when is_tuple(ip), do: :inet.ntoa(ip) |> to_string()
 
   defp update_via(%Message{start_line: %RequestLine{}} = request, {:wss, _ip, _from_port}), do: request
+
   defp update_via(%Message{start_line: %RequestLine{}} = request, {:ws, _ip, _from_port}), do: request
 
   defp update_via(%Message{start_line: %RequestLine{}} = request, {_protocol, ip, from_port}) do
@@ -231,8 +234,8 @@ defmodule Sippet.Router do
 
     initial_data = Transactions.Client.State.new(outgoing_request, key, sippet)
 
-    Supervisor.start_child(
-      :"#{sippet}_sup",
+    DynamicSupervisor.start_child(
+      supervisor_name(sippet),
       {module, [initial_data, [name: {:via, Registry, {sippet, {:transaction, key}}}]]}
     )
   end
@@ -250,8 +253,8 @@ defmodule Sippet.Router do
 
     initial_data = Transactions.Server.State.new(incoming_request, key, sippet)
 
-    Supervisor.start_child(
-      :"#{sippet}_sup",
+    DynamicSupervisor.start_child(
+      supervisor_name(sippet),
       {module, [initial_data, [name: {:via, Registry, {sippet, {:transaction, key}}}]]}
     )
   end

mix.lock

@@ -0,0 +1,33 @@
+%{
+  "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
+  "certifi": {:hex, :certifi, "2.8.0", "d4fb0a6bb20b7c9c3643e22507e42f356ac090a1dcea9ab99e27e0376d695eba", [:rebar3], [], "hexpm", "6ac7efc1c6f8600b08d625292d4bbf584e14847ce1b6b5c44d983d273e1097ea"},
+  "credo": {:hex, :credo, "1.6.2", "2f82b29a47c0bb7b72f023bf3a34d151624f1cbe1e6c4e52303b05a11166a701", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "ae9dc112bc368e7b145c547bec2ed257ef88955851c15057c7835251a17211c6"},
+  "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"},
+  "earmark": {:hex, :earmark, "1.4.3", "364ca2e9710f6bff494117dbbd53880d84bebb692dafc3a78eb50aa3183f2bfd", [:mix], [], "hexpm", "8cf8a291ebf1c7b9539e3cddb19e9cef066c2441b1640f13c34c1d3cfc825fec"},
+  "earmark_parser": {:hex, :earmark_parser, "1.4.19", "de0d033d5ff9fc396a24eadc2fcf2afa3d120841eb3f1004d138cbf9273210e8", [:mix], [], "hexpm", "527ab6630b5c75c3a3960b75844c314ec305c76d9899bb30f71cb85952a9dc45"},
+  "elixir_make": {:hex, :elixir_make, "0.6.3", "bc07d53221216838d79e03a8019d0839786703129599e9619f4ab74c8c096eac", [:mix], [], "hexpm", "f5cbd651c5678bcaabdbb7857658ee106b12509cd976c2c2fca99688e1daf716"},
+  "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
+  "ex_doc": {:hex, :ex_doc, "0.27.3", "d09ed7ab590b71123959d9017f6715b54a448d76b43cf909eb0b2e5a78a977b2", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "ee60b329d08195039bfeb25231a208749be4f2274eae42ce38f9be0538a2f2e6"},
+  "excoveralls": {:hex, :excoveralls, "0.14.4", "295498f1ae47bdc6dce59af9a585c381e1aefc63298d48172efaaa90c3d251db", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "e3ab02f2df4c1c7a519728a6f0a747e71d7d6e846020aae338173619217931c1"},
+  "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
+  "gen_state_machine": {:hex, :gen_state_machine, "3.0.0", "1e57f86a494e5c6b14137ebef26a7eb342b3b0070c7135f2d6768ed3f6b6cdff", [:mix], [], "hexpm", "0a59652574bebceb7309f6b749d2a41b45fdeda8dbb4da0791e355dd19f0ed15"},
+  "hackney": {:hex, :hackney, "1.18.0", "c4443d960bb9fba6d01161d01cd81173089686717d9490e5d3606644c48d121f", [:rebar3], [{:certifi, "~>2.8.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~>6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~>1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~>1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "9afcda620704d720db8c6a3123e9848d09c87586dc1c10479c42627b905b5c5e"},
+  "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
+  "inch_ex": {:hex, :inch_ex, "2.0.0", "24268a9284a1751f2ceda569cd978e1fa394c977c45c331bb52a405de544f4de", [:mix], [{:bunt, "~> 0.2", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "96d0ec5ecac8cf63142d02f16b7ab7152cf0f0f1a185a80161b758383c9399a8"},
+  "jason": {:hex, :jason, "1.3.0", "fa6b82a934feb176263ad2df0dbd91bf633d4a46ebfdffea0c8ae82953714946", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "53fc1f51255390e0ec7e50f9cb41e751c260d065dcba2bf0d08dc51a4002c2ac"},
+  "makeup": {:hex, :makeup, "1.0.5", "d5a830bc42c9800ce07dd97fa94669dfb93d3bf5fcf6ea7a0c67b2e0e4a7f26c", [:mix], [{:nimble_parsec, "~> 0.5 or ~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cfa158c02d3f5c0c665d0af11512fed3fba0144cf1aadee0f2ce17747fba2ca9"},
+  "makeup_elixir": {:hex, :makeup_elixir, "0.15.2", "dc72dfe17eb240552857465cc00cce390960d9a0c055c4ccd38b70629227e97c", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.1", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "fd23ae48d09b32eff49d4ced2b43c9f086d402ee4fd4fcb2d7fad97fa8823e75"},
+  "makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"},
+  "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"},
+  "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
+  "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
+  "mock": {:hex, :mock, "0.3.7", "75b3bbf1466d7e486ea2052a73c6e062c6256fb429d6797999ab02fa32f29e03", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "4da49a4609e41fd99b7836945c26f373623ea968cfb6282742bcb94440cf7e5c"},
+  "nimble_parsec": {:hex, :nimble_parsec, "1.2.0", "b44d75e2a6542dcb6acf5d71c32c74ca88960421b6874777f79153bbbbd7dccc", [:mix], [], "hexpm", "52b2871a7515a5ac49b00f214e4165a40724cf99798d8e4a65e4fd64ebd002c1"},
+  "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
+  "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"},
+  "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm", "451d8527787df716d99dc36162fca05934915db0b6141bbdac2ea8d3c7afc7d7"},
+  "sippet_uri": {:hex, :sippet_uri, "0.1.0", "ec8bfe6df12e17c0ca32b18b2177585426448bf3e3b0434f5ddfb93dc075add2", [:mix], [], "hexpm", "e8b1ad3fdd12670b4e9851a273624a7ed4b12cf5e705dec7906204c294d9790b"},
+  "socket": {:hex, :socket, "0.3.13", "98a2ab20ce17f95fb512c5cadddba32b57273e0d2dba2d2e5f976c5969d0c632", [:mix], [], "hexpm", "f82ea9833ef49dde272e6568ab8aac657a636acb4cf44a7de8a935acb8957c2e"},
+  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
+  "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
+}

test/sippet_router_test.exs

@@ -11,7 +11,7 @@ defmodule Sippet.Router.Test do
        [
          lookup: fn _, _ -> [] end
        ]},
-      {Supervisor, [],
+      {DynamicSupervisor, [],
        [
          start_child: fn _, {_, [%State{request: %{body: ""}}, _]} ->
            {:ok, self()}
@@ -37,8 +37,8 @@ defmodule Sippet.Router.Test do
       assert called(Registry.lookup(:sippet, {:transaction, :_}))
 
       assert called(
-               Supervisor.start_child(
-                 :sippet_sup,
+               DynamicSupervisor.start_child(
+                 Sippet.supervisor_name(:sippet),
                  {Sippet.Transactions.Server.NonInvite, :_}
                )
              )
@@ -61,7 +61,7 @@ defmodule Sippet.Router.Test do
        [
          lookup: fn _, _ -> [] end
        ]},
-      {Supervisor, [],
+      {DynamicSupervisor, [],
        [
          start_child: fn _, {_, [%State{request: %{body: @test_body}}, _]} ->
            {:ok, self()}
@@ -90,8 +90,8 @@ defmodule Sippet.Router.Test do
       assert called(Registry.lookup(:sippet, {:transaction, :_}))
 
       assert called(
-               Supervisor.start_child(
-                 :sippet_sup,
+               DynamicSupervisor.start_child(
+                 Sippet.supervisor_name(:sippet),
                  {Sippet.Transactions.Server.Invite, :_}
                )
              )
@@ -104,7 +104,7 @@ defmodule Sippet.Router.Test do
        [
          lookup: fn _, _ -> [] end
        ]},
-      {Supervisor, [],
+      {DynamicSupervisor, [],
        [
          start_child: fn _, _ -> {:ok, self()} end
        ]}
@@ -133,7 +133,7 @@ defmodule Sippet.Router.Test do
        [
          lookup: fn _, _ -> [] end
        ]},
-      {Supervisor, [],
+      {DynamicSupervisor, [],
        [
          start_child: fn _, _ -> {:ok, self()} end
        ]}

test/transaction_client_invite_test.exs

@@ -180,7 +180,7 @@ defmodule Sippet.Transactions.Client.Invite.Test do
       {:keep_state, data, actions} =
         Invite.completed(:enter, :proceeding, %{state | extras: extras})
 
-      assert action_timeout(actions, 32000)
+      assert action_timeout(actions, 32_000)
 
       %{extras: %{ack: ack}} = data
       assert :ack == ack.start_line.method