OTP-8529: Raise condition processing replies

5 files changed, 181 insertions, 38 deletions

diff --git a/lib/megaco/doc/src/notes.xml b/lib/megaco/doc/src/notes.xml
index 9963359515..99a3784402 100644
--- a/lib/megaco/doc/src/notes.xml
+++ b/lib/megaco/doc/src/notes.xml
@@ -66,6 +66,16 @@
 
       <list type="bulleted">
         <item>
+          <p>A raise condition when, during high load, processing
+            both the original and a resent message and delivering
+            this as two separate messages to the user. </p>
+          <p>Note that this solution only protects against multiple
+            reply deliveries! </p>
+          <p>Own Id: OTP-8529</p>
+          <p>Aux Id: Seq 10915</p>
+        </item>
+
+        <item>
           <p>Fix shared libraries installation. </p>
           <p>The flex shared lib(s) were incorrectly installed as data 
             files. </p>
diff --git a/lib/megaco/src/app/megaco.appup.src b/lib/megaco/src/app/megaco.appup.src
index 5df31f2923..f939f5e6cf 100644
--- a/lib/megaco/src/app/megaco.appup.src
+++ b/lib/megaco/src/app/megaco.appup.src
@@ -133,13 +133,16 @@
  [
   {"3.14", 
    [
+    {load_module, megaco_messenger, soft_purge, soft_purge, [megaco_monitor]},
+    {update,      megaco_monitor, soft, soft_purge, soft_purge, []}, 
     {update, megaco_config, soft, soft_purge, soft_purge, []}
    ]
   }, 
   {"3.13", 
    [
-    {load_module, megaco_messenger, soft_purge, soft_purge, []}, 
+    {load_module, megaco_messenger, soft_purge, soft_purge, [megaco_monitor]}, 
     {load_module, megaco_filter, soft_purge, soft_purge, []}, 
+    {update, megaco_monitor, soft, soft_purge, soft_purge, []}, 
     {update, megaco_config, soft, soft_purge, soft_purge, []}, 
     {update, megaco_flex_scanner_handler, {advanced, downgrade_to_pre_3_13_1}, 
      soft_purge, soft_purge, []}
@@ -173,13 +176,16 @@
  [
   {"3.14", 
    [
+    {load_module, megaco_messenger, soft_purge, soft_purge, [megaco_monitor]},
+    {update,      megaco_monitor, soft, soft_purge, soft_purge, []}, 
     {update, megaco_config, soft, soft_purge, soft_purge, []}
    ]
   }, 
   {"3.13", 
    [
-    {load_module, megaco_messenger, soft_purge, soft_purge, []}, 
+    {load_module, megaco_messenger, soft_purge, soft_purge, [megaco_monitor]}, 
     {load_module, megaco_filter, soft_purge, soft_purge, []}, 
+    {update,      megaco_monitor, soft, soft_purge, soft_purge, []}, 
     {update, megaco_config, soft, soft_purge, soft_purge, []}, 
     {update, megaco_flex_scanner_handler, {advanced, upgrade_from_pre_3_13_1}, 
      soft_purge, soft_purge, []}
diff --git a/lib/megaco/src/engine/megaco_messenger.erl b/lib/megaco/src/engine/megaco_messenger.erl
index d6e0918a31..5fad29931b 100644
--- a/lib/megaco/src/engine/megaco_messenger.erl
+++ b/lib/megaco/src/engine/megaco_messenger.erl
@@ -2607,33 +2607,84 @@ handle_reply(
 handle_reply(#conn_data{conn_handle = CH} = CD, T, Extra) ->
     TransId = to_local_trans_id(CD),
     ?rt2("handle reply", [T, TransId]),
-    case megaco_monitor:lookup_request(TransId) of
-	[Req] when (is_record(Req, request) andalso 
-		    (CD#conn_data.cancel =:= true)) -> 
+    case {megaco_monitor:request_lockcnt_inc(TransId),
+	  megaco_monitor:lookup_request(TransId)} of
+	{_Cnt, [Req]} when (is_record(Req, request) andalso 
+			    (CD#conn_data.cancel =:= true)) -> 
 	    ?TC_AWAIT_REPLY_EVENT(true),
+	    ?report_trace(CD, "trans reply - cancel(1)", [T]),
 	    do_handle_reply_cancel(CD, Req, T);
 
-	[#request{remote_mid = RMid} = Req] when ((RMid =:= preliminary_mid) orelse 
-						  (RMid =:= CH#megaco_conn_handle.remote_mid)) -> 
+	{Cnt, [#request{remote_mid = RMid} = Req]} when 
+	((Cnt =:= 1) andalso 
+	 ((RMid =:= preliminary_mid) orelse 
+	  (RMid =:= CH#megaco_conn_handle.remote_mid))) -> 
+	    ?TC_AWAIT_REPLY_EVENT(false),
+	    %% Just in case conn_data got update after our lookup
+	    %% but before we looked up the request record, we
+	    %% check the cancel field again.
+	    case megaco_config:conn_info(CD, cancel) of
+		true ->
+		    ?report_trace(CD, "trans reply - cancel(2)", [T]),
+		    megaco_monitor:request_lockcnt_del(TransId), 
+		    do_handle_reply_cancel(CD, Req, T);
+		false ->
+		    ?report_trace(CD, "trans reply", [T]),
+		    do_handle_reply(CD, Req, TransId, T, Extra)
+	    end;
+
+	{Cnt, [#request{remote_mid = RMid} = _Req]} when 
+	(is_integer(Cnt) andalso 
+	 ((RMid =:= preliminary_mid) orelse 
+	  (RMid =:= CH#megaco_conn_handle.remote_mid))) -> 
 	    ?TC_AWAIT_REPLY_EVENT(false),
+	    %% Ok, someone got there before me, now what?
+	    %% This is a plain old raise condition
+	    ?report_important(CD, "trans reply - raise condition", 
+			      [T, {request_lockcnt, Cnt}]),
+	    megaco_monitor:request_lockcnt_dec(TransId);
+
+	%% no counter
+	{_Cnt, [#request{remote_mid = RMid} = Req]} when 
+	 ((RMid =:= preliminary_mid) orelse 
+	  (RMid =:= CH#megaco_conn_handle.remote_mid)) -> 
+	    ?TC_AWAIT_REPLY_EVENT(false),
+	    %% The counter does not exist. 
+	    %% This can only mean a code upgrade raise condition.
+	    %% That is, this request record was created before 
+	    %% this feature (the counters) was instroduced.
+	    %% The simples solution is this is to behave exactly as
+	    %% before, that is pass it along, and leave it to the 
+	    %% user to figure out.
+
 	    %% Just in case conn_data got update after our lookup
 	    %% but before we looked up the request record, we
 	    %% check the cancel field again.
+	    ?report_verbose(CD, "trans reply - old style", [T]),
 	    case megaco_config:conn_info(CD, cancel) of
 		true ->
+		    megaco_monitor:request_lockcnt_del(TransId), 
 		    do_handle_reply_cancel(CD, Req, T);
 		false ->
 		    do_handle_reply(CD, Req, TransId, T, Extra)
 	    end;
 
-	[#request{user_mod     = UserMod,
-		  user_args    = UserArgs, 
-		  reply_action = Action, 
-		  reply_data   = UserData,
-		  remote_mid   = RMid}] ->
+	{Cnt, [#request{user_mod     = UserMod,
+			user_args    = UserArgs, 
+			reply_action = Action, 
+			reply_data   = UserData,
+			remote_mid   = RMid}]} ->
 	    ?report_trace(CD, 
 			  "received trans reply with invalid remote mid", 
-			  [T, RMid]),
+			  [{transaction,     T}, 
+			   {remote_mid,      RMid}, 
+			   {request_lockcnt, Cnt}]),
+	    if
+		is_integer(Cnt) ->
+		    megaco_monitor:request_lockcnt_dec(TransId);
+		true ->
+		    ok
+	    end,
 	    WrongMid = CH#megaco_conn_handle.remote_mid,
 	    T2 = transform_transaction_reply_enc(CD#conn_data.protocol_version,
 						 T),
@@ -2644,7 +2695,15 @@ handle_reply(#conn_data{conn_handle = CH} = CD, T, Extra) ->
 			       reply_data   = UserData},
 	    return_reply(CD2, TransId, UserReply, Extra);
 
-	[] ->
+	{Cnt, []} when is_integer(Cnt) ->
+	    ?TC_AWAIT_REPLY_EVENT(undefined),
+	    ?report_trace(CD, "trans reply (no receiver)", 
+			  [T, {request_lockcnt, Cnt}]),
+	    megaco_monitor:request_lockcnt_dec(TransId),
+	    return_unexpected_trans(CD, T, Extra);
+
+	%% No counter
+	{_Cnt, []} -> 
 	    ?TC_AWAIT_REPLY_EVENT(undefined),
 	    ?report_trace(CD, "trans reply (no receiver)", [T]),
 	    return_unexpected_trans(CD, T, Extra)
@@ -2675,6 +2734,7 @@ do_handle_reply(CD,
 
     %% This is the first reply (maybe of many)
     megaco_monitor:delete_request(TransId),
+    megaco_monitor:request_lockcnt_del(TransId), 
     megaco_monitor:cancel_apply_after(Ref),           % OTP-4843
     megaco_config:del_pending_counter(recv, TransId), % OTP-7189
 
@@ -3698,6 +3758,11 @@ insert_requests(ConnData, ConnHandle,
 
 insert_request(ConnData, ConnHandle, TransId, 
 	       Action, Data, InitTimer, LongTimer) ->
+    %% We dont check the result of the lock-counter creation because
+    %% the only way it could already exist is if the transaction-id
+    %% range has wrapped and an old counter was not deleted.
+    megaco_monitor:request_lockcnt_cre(TransId),
+
     #megaco_conn_handle{remote_mid = RemoteMid} = ConnHandle,
     #conn_data{protocol_version           = Version,
 	       user_mod                   = UserMod,
@@ -4282,6 +4347,7 @@ cancel_request(ConnData, Req, Reason)  ->
 
 cancel_request2(ConnData, TransId, UserReply) ->
     megaco_monitor:delete_request(TransId),
+    megaco_monitor:request_lockcnt_del(TransId), 
     megaco_config:del_pending_counter(recv, TransId), % OTP-7189
     Serial    = TransId#trans_id.serial,
     ConnData2 = ConnData#conn_data{serial = Serial},
@@ -4339,29 +4405,67 @@ receive_reply_remote(ConnData, UserReply) ->
 
 receive_reply_remote(ConnData, UserReply, Extra) ->
     TransId = to_local_trans_id(ConnData),
-    case (catch megaco_monitor:lookup_request(TransId)) of
-        [#request{timer_ref = {_Type, Ref}} = Req] -> %% OTP-4843
+    case {megaco_monitor:request_lockcnt_inc(TransId),
+	  (catch megaco_monitor:lookup_request(TransId))} of
+        {Cnt, [Req]} when (Cnt =:= 1) andalso is_record(Req, request) -> 
             %% Don't care about Req and Rep version diff
-	    megaco_monitor:delete_request(TransId),
-	    megaco_monitor:cancel_apply_after(Ref),           % OTP-4843
-	    megaco_config:del_pending_counter(recv, TransId), % OTP-7189
-	
-	    UserMod   = Req#request.user_mod,
-	    UserArgs  = Req#request.user_args,
-	    Action    = Req#request.reply_action,
-	    UserData  = Req#request.reply_data,
-	    ConnData2 = ConnData#conn_data{user_mod     = UserMod,
-					   user_args    = UserArgs,
-					   reply_action = Action,
-					   reply_data   = UserData},
-	    return_reply(ConnData2, TransId, UserReply, Extra);
-		
+	    do_receive_reply_remote(ConnData, TransId, Req, UserReply, Extra);
+
+        {Cnt, [Req]} when is_integer(Cnt) andalso is_record(Req, request) -> 
+            %% Another process is accessing, handle as unexpected 
+	    %% (so it has a possibillity to get logged).
+	    ?report_important(ConnData, "trans reply (no receiver)", 
+			      [{user_reply,      UserReply}, 
+			       {request_lockcnt, Cnt}]),
+	    megaco_monitor:request_lockcnt_dec(TransId),
+	    return_unexpected_trans_reply(ConnData, TransId, UserReply, Extra);
+
+	%% no counter
+        {_Cnt, [Req]} when is_record(Req, request) -> 
+            %% The counter does not exist. 
+	    %% This can only mean a code upgrade raise condition.
+	    %% That is, this request record was created before 
+	    %% this feature (the counters) was instroduced.
+	    %% The simples solution to this is to behave exactly as
+	    %% before, that is, pass it along, and leave it to the 
+	    %% user to figure out.
+	    ?report_trace(ConnData, 
+			  "remote reply - "
+			  "code upgrade raise condition", 
+			  [{user_reply, UserReply}]),
+	    do_receive_reply_remote(ConnData, TransId, Req, UserReply, Extra);
+
+	{Cnt, _} when is_integer(Cnt) ->
+	    ?report_trace(ConnData, "trans reply (no receiver)", 
+			  [{user_reply, UserReply}, {request_lockcnt, Cnt}]),
+	    megaco_monitor:request_lockcnt_dec(TransId), 
+	    return_unexpected_trans_reply(ConnData, TransId, UserReply, Extra);
+
 	_ ->
 	    ?report_trace(ConnData, "remote reply (no receiver)", 
-			  [UserReply]),
+			  [{user_reply, UserReply}]),
 	    return_unexpected_trans_reply(ConnData, TransId, UserReply, Extra)
     end.
 
+do_receive_reply_remote(ConnData, TransId, 
+			#request{timer_ref    = {_Type, Ref},
+				 user_mod     = UserMod, 
+				 user_args    = UserArgs,
+				 reply_action = Action,
+				 reply_data   = UserData} = _Req, 
+			UserReply, Extra) ->
+    megaco_monitor:delete_request(TransId),
+    megaco_monitor:request_lockcnt_del(TransId), 
+    megaco_monitor:cancel_apply_after(Ref),           % OTP-4843
+    megaco_config:del_pending_counter(recv, TransId), % OTP-7189
+
+    ConnData2 = ConnData#conn_data{user_mod     = UserMod,
+				   user_args    = UserArgs,
+				   reply_action = Action,
+				   reply_data   = UserData},
+    return_reply(ConnData2, TransId, UserReply, Extra).
+
+
 cancel_reply(ConnData, #reply{state     = waiting_for_ack,
 			      user_mod  = UserMod,
 			      user_args = UserArgs} = Rep, Reason) ->
diff --git a/lib/megaco/src/engine/megaco_monitor.erl b/lib/megaco/src/engine/megaco_monitor.erl
index f95a20cf58..29275371be 100644
--- a/lib/megaco/src/engine/megaco_monitor.erl
+++ b/lib/megaco/src/engine/megaco_monitor.erl
@@ -1,19 +1,19 @@
 %%
 %% %CopyrightBegin%
-%% 
-%% Copyright Ericsson AB 2000-2009. All Rights Reserved.
-%% 
+%%
+%% Copyright Ericsson AB 2000-2010. All Rights Reserved.
+%%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
 %% compliance with the License. You should have received a copy of the
 %% Erlang Public License along with this software. If not, it can be
 %% retrieved online at http://www.erlang.org/.
-%% 
+%%
 %% Software distributed under the License is distributed on an "AS IS"
 %% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
 %% the License for the specific language governing rights and limitations
 %% under the License.
-%% 
+%%
 %% %CopyrightEnd%
 %%
 
@@ -51,6 +51,11 @@
 	 update_request_field/3, update_request_fields/2, 
 	 delete_request/1, 
 
+	 request_lockcnt_cre/1, 
+	 request_lockcnt_del/1, 
+	 request_lockcnt_inc/1, 
+	 request_lockcnt_dec/1, 
+
 	 lookup_reply/1,
 	 lookup_reply_field/2,
 	 match_replies/1,
@@ -115,6 +120,24 @@ update_request_fields(Key, NewFields) when is_list(NewFields) ->
 delete_request(Key) ->
     ets:delete(megaco_requests, Key).
 
+
+request_lockcnt_cre(TransId) ->
+    Key = {TransId, lockcnt},
+    ets:insert_new(megaco_requests, {Key, 1}).
+
+request_lockcnt_del(TransId) ->
+    Key = {TransId, lockcnt},
+    ets:delete(megaco_requests, Key).
+
+request_lockcnt_inc(TransId) ->
+    Key = {TransId, lockcnt},
+    (catch ets:update_counter(megaco_requests, Key, 1)).
+
+request_lockcnt_dec(TransId) ->
+    Key = {TransId, lockcnt},
+    (catch ets:update_counter(megaco_requests, Key, -1)).
+
+
 lookup_reply(Key) ->
     ets:lookup(megaco_replies, Key).
 
diff --git a/lib/megaco/vsn.mk b/lib/megaco/vsn.mk
index 9904824382..cab3a1a4e0 100644
--- a/lib/megaco/vsn.mk
+++ b/lib/megaco/vsn.mk
@@ -19,10 +19,10 @@
 
 APPLICATION = megaco
 MEGACO_VSN = 3.14.1
-PRE_VSN    =-p02
+PRE_VSN    =-p03
 APP_VSN    = "$(APPLICATION)-$(MEGACO_VSN)$(PRE_VSN)"
 
-TICKETS = OTP-8561 OTP-8627 OTP-8634
+TICKETS = OTP-8529 OTP-8561 OTP-8627 OTP-8634
 
 TICKETS_3_14 = OTP-8317 OTP-8323 OTP-8328 OTP-8362 OTP-8403