Fix race condition in starting/stopping farmware

farmbot_core/lib/farmbot_core/farmware_runtime.ex

@@ -77,6 +77,12 @@ defmodule FarmbotCore.FarmwareRuntime do
     GenServer.start_link(__MODULE__, [manifest, env], name: String.to_atom(package))
   end
 
+  @doc "Stop a farmware"
+  def stop(pid) do
+    Logger.info "Terminating farmware process"
+    GenServer.stop(pid, :normal)
+  end
+
   def init([manifest, env]) do
     package = manifest.package
     <<clause1 :: binary-size(8), _::binary>> = Ecto.UUID.generate()
@@ -185,7 +191,7 @@ defmodule FarmbotCore.FarmwareRuntime do
   # farmware exit
   def handle_info({:DOWN, _ref, :process, _pid, _reason}, %{cmd: _cmd_pid} = state) do
     Logger.debug("Farmware exit")
-    {:stop, :normal, state}
+    {:noreply, %{state | cmd: nil}}
   end
 
   # successful result of an io:read/2 in :get_header context

farmbot_os/lib/farmbot_os/sys_calls/farmware.ex

@@ -21,8 +21,10 @@ defmodule FarmbotOS.SysCalls.Farmware do
          :ok <- ImageUploader.force_checkup() do
       :ok
     else
-      {:error, {:already_started, _pid}} ->
-        {:error, "Farmware #{farmware_name} is already running"}
+      {:error, {:already_started, pid}} ->
+        Logger.warn("Farmware #{farmware_name} is already running")
+        _ = FarmwareRuntime.stop(pid)
+        execute_script(farmware_name, env)
 
       {:error, reason} when is_binary(reason) ->
         _ = ImageUploader.force_checkup()
@@ -46,10 +48,10 @@ defmodule FarmbotOS.SysCalls.Farmware do
 
   defp loop(farmware_name, runtime, monitor, {ref, label}) do
     receive do
-      {:DOWN, ^monitor, :process, _runtime, :normal} ->
+      {:DOWN, ^monitor, :process, ^runtime, :normal} ->
         :ok
 
-      {:DOWN, ^monitor, :process, _runtime, error} ->
+      {:DOWN, ^monitor, :process, ^runtime, error} ->
         {:error, inspect(error)}
 
       {:step_complete, ^ref, :ok} ->
@@ -66,26 +68,33 @@ defmodule FarmbotOS.SysCalls.Farmware do
         loop(farmware_name, runtime, monitor, {nil, nil})
 
       msg ->
+        _ = FarmwareRuntime.stop(runtime)
         {:error, "unhandled message: #{inspect(msg)} in state: #{inspect({ref, label})}"}
     after
       500 ->
-        if is_reference(ref) do
-          Logger.info("Already processing a celeryscript request: #{label}")
-          loop(farmware_name, runtime, monitor, {ref, label})
-        else
-          case Process.alive?(runtime) && FarmwareRuntime.process_rpc(runtime) do
-            {:ok, %{args: %{label: label}} = rpc} ->
-              ref = make_ref()
-              Logger.debug("executing rpc: #{inspect(rpc)}")
-              FarmbotCeleryScript.execute(rpc, ref)
-              loop(farmware_name, runtime, monitor, {ref, label})
+        cond do
+          # already have a request processing
+          is_reference(ref) ->
+            Logger.info("Already processing a celeryscript request: #{label}")
+            loop(farmware_name, runtime, monitor, {ref, label})
 
-            {:error, :no_rpc} ->
-              loop(farmware_name, runtime, monitor, {ref, label})
+          # check to see if it's alive just in case?
+          Process.alive?(runtime) ->
+            case FarmwareRuntime.process_rpc(runtime) do
+              {:ok, %{args: %{label: label}} = rpc} ->
+                ref = make_ref()
+                Logger.debug("executing rpc: #{inspect(rpc)}")
+                FarmbotCeleryScript.execute(rpc, ref)
+                loop(farmware_name, runtime, monitor, {ref, label})
 
-            false ->
-              :ok
-          end
+              {:error, :no_rpc} ->
+                loop(farmware_name, runtime, monitor, {ref, label})
+            end
+
+          # No other conditions: Process stopped, but missed the message?
+          true ->
+            _ = FarmwareRuntime.stop(runtime)
+            :ok
         end
     end
   end