Merge remote-tracking branch 'origin/master' into remove-skill-from-hexdocs-sidebar

Author: FelonEkonom

lib/membrane/core/subprocess_supervisor.ex

@@ -139,7 +139,8 @@ defmodule Membrane.Core.SubprocessSupervisor do
           name: {__MODULE__, name},
           type: :supervisor,
           child_pid: child_pid,
-          role: :subprocess_supervisor
+          role: :subprocess_supervisor,
+          pending_child_death: nil
         })
 
       {:reply, {:ok, child_pid}, state}
@@ -221,7 +222,7 @@ defmodule Membrane.Core.SubprocessSupervisor do
 
   defp do_handle_info({:EXIT, pid, reason}, state) do
     {data, state} = pop_in(state, [:children, pid])
-    handle_exit(data, reason, state)
+    state = handle_exit(data, reason, state)
 
     case state do
       %{parent_process: :exit_requested, children: children} when children == %{} ->
@@ -245,27 +246,33 @@ defmodule Membrane.Core.SubprocessSupervisor do
   end
 
   defp handle_exit(%{role: :subprocess_supervisor} = data, reason, state) do
-    case Map.fetch(state.children, data.child_pid) do
+    with :error <- Map.fetch(state.children, data.child_pid),
+         %{pending_child_death: {child_name, death_reason}} <- data do
+      Message.send(state.parent_component, :child_death, [child_name, death_reason])
+    else
       {:ok, child_data} ->
         raise "Subprocess supervisor failure #{inspect(child_data.name)}, reason: #{inspect(reason)}"
 
-      :error ->
+      _no_pending_death ->
         :ok
     end
+
+    state
   end
 
   defp handle_exit(%{role: :component} = data, reason, state) do
     Process.exit(data.supervisor_pid, :shutdown)
-    Message.send(state.parent_component, :child_death, [data.name, reason])
+
+    put_in(state.children[data.supervisor_pid].pending_child_death, {data.name, reason})
   end
 
-  defp handle_exit(%{role: :utility}, _reason, _state) do
-    :ok
+  defp handle_exit(%{role: :utility}, _reason, state) do
+    state
   end
 
   # Clause handling the case when child start function returns error
   # and we don't know its PID, but we still receive exit signal from it.
-  defp handle_exit(nil, _reason, _state) do
-    :ok
+  defp handle_exit(nil, _reason, state) do
+    state
   end
 end

test/membrane/integration/callbacks_test.exs

@@ -121,4 +121,61 @@ defmodule Membrane.Integration.CallbacksTest do
       {:DOWN, _ref, _process, ^pipeline, _reason} -> :ok
     end
   end
+
+  defmodule BinWithTwoChildren do
+    use Membrane.Bin
+
+    alias Membrane.Integration.CallbacksTest.PadlessElement
+
+    @impl true
+    def handle_init(_ctx, _opts) do
+      {[spec: [child(:child_a, PadlessElement), child(:child_b, PadlessElement)]], %{}}
+    end
+
+    @impl true
+    def handle_playing(ctx, state) do
+      {[notify_parent: {:children_pids, ctx.children.child_a.pid, ctx.children.child_b.pid}],
+       state}
+    end
+  end
+
+  defmodule BinTerminationPipeline do
+    use Membrane.Pipeline
+
+    @impl true
+    def handle_child_notification({:children_pids, a_pid, b_pid}, :bin, _ctx, state) do
+      {[], %{state | a_pid: a_pid, b_pid: b_pid}}
+    end
+
+    @impl true
+    def handle_child_terminated(:bin, _ctx, state) do
+      send(
+        state.test_pid,
+        {:terminated, Process.alive?(state.a_pid), Process.alive?(state.b_pid)}
+      )
+
+      {[], state}
+    end
+  end
+
+  test "handle_child_terminated fires only after bin's subprocess_supervisor dies, ensuring bin's children are already dead" do
+    pipeline =
+      Testing.Pipeline.start_link_supervised!(
+        module: BinTerminationPipeline,
+        custom_args: %{test_pid: self(), a_pid: nil, b_pid: nil}
+      )
+
+    Testing.Pipeline.execute_actions(pipeline,
+      spec: {child(:bin, BinWithTwoChildren), group: :bin_group, crash_group_mode: :temporary}
+    )
+
+    assert_pipeline_notified(pipeline, :bin, {:children_pids, _a_pid, _b_pid})
+
+    Testing.Pipeline.get_child_pid!(pipeline, :bin)
+    |> Process.exit(:crash)
+
+    assert_receive {:terminated, false, false}, 1000
+
+    Testing.Pipeline.terminate(pipeline)
+  end
 end