Fix memory leak in PDF cleanup

Author: andrewtimberlake

lib/pdf.ex

@@ -137,7 +137,6 @@ defmodule Pdf do
   end
 
   @deprecated "Use build/2 instead"
-  @spec open((any() -> any())) :: any()
   def open(opts \\ [], func) do
     build(opts, func)
   end
@@ -492,7 +491,7 @@ defmodule Pdf do
   `:align` | :left , :center , :right | :left
   `:kerning` | `boolean` | false
   """
-  @spec text_wrap(pid, coords(), dimension(), binary | list, keyword) :: pid
+  @spec text_wrap(pid, coords(), dimension(), binary | list, keyword) :: {pid, :complete | term()}
   def text_wrap(pid, coords, dimensions, text, opts) do
     result = GenServer.call(pid, {:text_wrap, coords, dimensions, text, opts})
     {pid, result}
@@ -657,12 +656,6 @@ defmodule Pdf do
     pid
   end
 
-  def terminate(_, %{objects: objects, fonts: fonts}) do
-    GenServer.stop(objects)
-    GenServer.stop(fonts)
-    nil
-  end
-
   defmodule Server do
     use GenServer
 
@@ -684,7 +677,7 @@ defmodule Pdf do
     end
 
     def handle_call(:export, _from, document) do
-      {:reply, Document.to_iolist(document) |> :binary.list_to_bin(), document}
+      {:reply, :binary.list_to_bin(Document.to_iolist(document)), document}
     end
 
     def handle_call({:add_page, size}, _from, document) do
@@ -836,5 +829,12 @@ defmodule Pdf do
     def handle_call({:set_info, key, value}, _from, document) do
       {:reply, :ok, Document.put_info(document, key, value)}
     end
+
+    @impl GenServer
+    def terminate(_, %{objects: objects, fonts: fonts}) do
+      GenServer.stop(objects)
+      GenServer.stop(fonts)
+      nil
+    end
   end
 end

test/pdf/document_test.exs

@@ -1,6 +1,5 @@
 defmodule Pdf.DocumentTest do
   use ExUnit.Case, async: true
-  import Pdf.Utils
 
   alias Pdf.{Document, ObjectCollection, Dictionary}
 

test/pdf_test.exs

@@ -197,4 +197,38 @@ defmodule PdfTest do
                    end
     end)
   end
+
+  test "cleanup" do
+    {:ok, pdf} = Pdf.new(size: :a4)
+    {:links, links} = Process.info(pdf, :links)
+    {:dictionary, dictionary} = Process.info(pdf, :dictionary)
+    ancestors = Keyword.get(dictionary, :"$ancestors")
+    # One of our links is an ancestor which will remain active after cleanup
+    children = links -- ancestors
+    # We expect an Object collection and a Fonts server to be linked
+    assert length(children) == 2
+
+    assert :ok = Pdf.cleanup(pdf)
+
+    assert Process.alive?(pdf) == false
+    refute Enum.any?(children, &Process.alive?/1)
+  end
+
+  test "crash cleanup" do
+    {:ok, pdf} = Pdf.new(size: :a4)
+    # Unlink so we can crash the PDF process without raising an error in the test
+    Process.unlink(pdf)
+    {:links, links} = Process.info(pdf, :links)
+    {:dictionary, dictionary} = Process.info(pdf, :dictionary)
+    ancestors = Keyword.get(dictionary, :"$ancestors")
+    # One of our links is an ancestor which will remain active after crash
+    children = links -- ancestors
+    # We expect an Object collection and a Fonts server to be linked
+    assert length(children) == 2
+
+    Process.exit(pdf, :kill)
+
+    assert Process.alive?(pdf) == false
+    refute Enum.any?(children, &Process.alive?/1)
+  end
 end