Browse Source

Fix bug when a member is returned more than once

In the event that the same member pid was returned multiple times -- in
essence "double freed" -- the pool accounting became corrupted.

When handling returns, we now ignore members that are already in the
free state and log a warning message since this is very likely to be an
indication of a problem in the calling code. Members should only be
free'd once.
Seth Falcon 10 years ago
parent
commit
83e7ad3b3d
2 changed files with 26 additions and 1 deletions
  1. 6 1
      src/pooler.erl
  2. 20 0
      test/pooler_tests.erl

+ 6 - 1
src/pooler.erl

@@ -499,9 +499,14 @@ add_members_async(Count, #pool{starting_members = StartingMembers} = Pool) ->
     Pool#pool{starting_members = StartRefs ++ StartingMembers}.
 
 -spec do_return_member(pid(), ok | fail, #pool{}) -> #pool{}.
-do_return_member(Pid, ok, #pool{all_members = AllMembers} = Pool) ->
+do_return_member(Pid, ok, #pool{name = PoolName,
+                                all_members = AllMembers} = Pool) ->
     clean_group_table(Pid, Pool),
     case dict:find(Pid, AllMembers) of
+        {ok, {_, free, _}} ->
+            Fmt = "pool '~s': ignored return of free member ~p",
+            error_logger:warning_msg(Fmt, [PoolName, Pid]),
+            Pool;
         {ok, {MRef, CPid, _}} ->
             #pool{free_pids = Free, in_use_count = NumInUse,
                   free_count = NumFree} = Pool,

+ 20 - 0
test/pooler_tests.erl

@@ -1,6 +1,7 @@
 -module(pooler_tests).
 
 -include_lib("eunit/include/eunit.hrl").
+-include("../src/pooler.hrl").
 
 -compile([export_all]).
 
@@ -292,6 +293,25 @@ basic_tests() ->
        end
       },
 
+      {"it is ok to return a pid more than once",
+       fun() ->
+               M = pooler:take_member(test_pool_1),
+               [ pooler:return_member(test_pool_1, M)
+                 || _I <- lists:seq(1, 37) ],
+               M1 = pooler:take_member(test_pool_1),
+               M2 = pooler:take_member(test_pool_1),
+               ?assert(M1 =/= M2),
+               Pool1 = gen_server:call(test_pool_1, dump_pool),
+               ?assertEqual(2, Pool1#pool.in_use_count),
+               ?assertEqual(0, Pool1#pool.free_count),
+               pooler:return_member(test_pool_1, M1),
+               pooler:return_member(test_pool_1, M2),
+               Pool2 = gen_server:call(test_pool_1, dump_pool),
+               ?assertEqual(0, Pool2#pool.in_use_count),
+               ?assertEqual(2, Pool2#pool.free_count),
+               ok
+       end},
+
       {"calling return_member on error_no_members is ignored",
        fun() ->
                ?assertEqual(ok, pooler:return_member(test_pool_1, error_no_members)),