Merge pull request basho#1 from zotonic/develop

Zotonic changes to side job

src/sidejob.erl

@@ -19,7 +19,7 @@
 %% -------------------------------------------------------------------
 -module(sidejob).
 -export([new_resource/3, new_resource/4, call/2, call/3, cast/2,
-         unbounded_cast/2]).
+         unbounded_cast/2, resource_exists/1]).
 
 %%%===================================================================
 %%% API
@@ -90,6 +90,20 @@ unbounded_cast(Name, Msg) ->
     Worker = preferred_worker(Name),
     gen_server:cast(Worker, Msg).
 
+%%% @doc
+%% Check if the specified resource exists. Erlang docs call out that
+%% using erlang:module_exists should not be used, so try to call
+%% a function on the module in question and, if it succeeds, return
+%% true. Otherwise, the module hasn't been created so return false.
+-spec resource_exists(Mod::atom()) -> boolean().
+resource_exists(Mod) ->
+    try
+        _ = Mod:width(),
+        true
+    catch _:_ ->
+        false
+    end.
+
 %%%===================================================================
 %%% Internal functions
 %%%===================================================================

src/sidejob_supervisor.erl

@@ -42,11 +42,15 @@
 
 -type resource() :: atom().
 
+-define(SPAWNED_JOB_WAIT_TIMEOUT, 5000).
+
 %%%===================================================================
 %%% API
 %%%===================================================================
 
 -spec start_child(resource(), module(), atom(), term()) -> {ok, pid()} |
+                                                           {ok, pid(), term()} |
+                                                           {ok, undefined} |
                                                            {error, overload} |
                                                            {error, term()}.
 start_child(Name, Mod, Fun, Args) ->
@@ -59,11 +63,12 @@ start_child(Name, Mod, Fun, Args) ->
 
 -spec spawn(resource(), function() | {module(), atom(), [term()]}) -> {ok, pid()} | {error, overload}.
 spawn(Name, Fun) ->
-    case sidejob:call(Name, {spawn, Fun}, infinity) of
+    case sidejob:call(Name, spawn, infinity) of
+        {ok, Pid} when is_pid(Pid) ->
+            Pid ! {f, Fun},
+            {ok, Pid};
         overload ->
-            {error, overload};
-        Other ->
-            Other
+            {error, overload}
     end.
 
 -spec spawn(resource(), module(), atom(), [term()]) -> {ok, pid()} |
@@ -104,33 +109,28 @@ handle_call({start_child, Mod, Fun, Args}, _From, State) ->
                       end,
     {reply, Reply, State2};
 
-handle_call({spawn, Fun}, _From, State) ->
-    Pid = case Fun of
-              _ when is_function(Fun) ->
-                  spawn_link(Fun);
-              {M, F, A} ->
-                  spawn_link(M, F, A)
-          end,
+handle_call(spawn, _From, State) ->
+    Pid = erlang:spawn_link(fun spawned_worker/0),
     State2 = add_child(Pid, State),
-    {reply, Pid, State2};
+    {reply, {ok, Pid}, State2};
 
 handle_call(_Request, _From, State) ->
     {reply, ok, State}.
 
 handle_cast(_Msg, State) ->
     {noreply, State}.
 
-handle_info({'EXIT', Pid, Reason}, State=#state{children=Children,
+handle_info({'EXIT', Pid, _Reason}, State=#state{children=Children,
                                                 died=Died}) ->
-    case sets:is_element(Pid, Children) of
+    State2 = case sets:is_element(Pid, Children) of
         true ->
             Children2 = sets:del_element(Pid, Children),
             Died2 = Died + 1,
-            State2 = State#state{children=Children2, died=Died2},
-            {noreply, State2};
+            State#state{children=Children2, died=Died2};
         false ->
-            {stop, Reason, State}
-    end;
+            State
+    end,
+    {noreply, State2};
 
 handle_info(_Info, State) ->
     {noreply, State}.
@@ -155,6 +155,17 @@ rate(State=#state{spawned=Spawned, died=Died}) ->
 %%% Internal functions
 %%%===================================================================
 
+spawned_worker() ->
+    receive
+        {f, {M,F,A}} ->
+            erlang:apply(M,F,A);
+        {f, Fun} ->
+            Fun()
+    after
+        ?SPAWNED_JOB_WAIT_TIMEOUT ->
+            erlang:exit(wait_timeout)
+    end.
+
 add_child(Pid, State=#state{children=Children, spawned=Spawned}) ->
     Children2 = sets:add_element(Pid, Children),
     Spawned2 = Spawned + 1,

test/supervisor_test.erl

@@ -0,0 +1,21 @@
+-module(supervisor_test).
+-export([fail_to_start_link/0]).
+-include_lib("eunit/include/eunit.hrl").
+
+-define(RESOURCE, supervisor_test_resource).
+
+% Setup a sidejob resource and start a child that links to the parent but doesn't return {ok, Pid}.
+% The supervisor should not shut down.
+spurious_exit_test() ->
+  {ok, _} = application:ensure_all_started(sidejob),
+  {ok, _} = sidejob:new_resource(?RESOURCE, sidejob_supervisor, 5, 1),
+  {WorkerReg} = ?RESOURCE:workers(),
+  WorkerPid = whereis(WorkerReg),
+  ?assert(is_process_alive(WorkerPid)),
+  {ok, undefined} = sidejob_supervisor:start_child(?RESOURCE, ?MODULE, fail_to_start_link, []),
+  sidejob_supervisor:which_children(?RESOURCE), % wait for sidejob_supervisor to process the EXIT
+  ?assert(is_process_alive(WorkerPid)).
+
+fail_to_start_link() ->
+  spawn_link(fun () -> ok end),
+  ignore.