megaco: Patch 1123

    OTP-8561  A minor compiler related performance improvement.

    OTP-8529  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. Note that this solution
              only protects against multiple reply deliveries!

    OTP-8627  Fix shared libraries installation. The flex shared lib(s)
              were incorrectly installed as data files.

    OTP-8634  Eliminated a possible raise condition while creating pending
              counters.

1 files changed, 151 insertions, 88 deletions

diff --git a/lib/megaco/src/engine/megaco_messenger.erl b/lib/megaco/src/engine/megaco_messenger.erl
index 5756e8e896..5fad29931b 100644
--- a/lib/megaco/src/engine/megaco_messenger.erl
+++ b/lib/megaco/src/engine/megaco_messenger.erl
@@ -1541,30 +1541,6 @@ check_pending_limit(Limit, Direction, TransId) ->
 	    aborted
     end.
 
-%% check_pending_limit(infinity, _, _) ->
-%%     {ok, 0};
-%% check_pending_limit(Limit, Direction, TransId) ->
-%%     ?rt2("check pending limit", [Direction, Limit, TransId]),
-%%     case (catch megaco_config:get_pending_counter(Direction, TransId)) of
-%% 	{'EXIT', _} ->
-%% 	    %% This function is only called when we "know" the 
-%% 	    %% counter to exist. So, the only reason that this 
-%% 	    %% would happen is of the counter has been removed.
-%% 	    %% This only happen if the pending limit has been 
-%% 	    %% reached. In any case, this is basically the same 
-%% 	    %% as aborted!
-%% 	    ?rt2("check pending limit - exit", []),
-%% 	    aborted;
-%% 	Val when Val =< Limit ->
-%% 	    %% Since we have no intention to increment here, it
-%% 	    %% is ok to be _at_ the limit
-%% 	    ?rt2("check pending limit - ok", [Val]),
-%% 	    {ok, Val};
-%% 	_Val ->
-%% 	    ?rt2("check pending limit - aborted", [_Val]),
-%% 	    aborted
-%%     end.
-
 
 check_and_maybe_incr_pending_limit(infinity, _, _) ->
     ok;
@@ -1572,59 +1548,42 @@ check_and_maybe_incr_pending_limit(Limit, Direction, TransId) ->
     %% 
     %% We need this kind of test to detect when we _pass_ the limit
     %% 
-    ?rt2("check and maybe incr pending limit", [Direction, Limit, TransId]),
+    ?rt2("check and maybe incr pending limit", [{direction,      Direction}, 
+						{transaction_id, TransId}, 
+						{counter_limit,  Limit}]),
     try megaco_config:get_pending_counter(Direction, TransId) of
 	Val when Val > Limit ->
-	    ?rt2("check and maybe incr - aborted", [Direction, Val, Limit]),
+	    ?rt2("check and maybe incr - aborted", [{counter_value, Val}]),
 	    aborted;      % Already passed the limit
 	Val ->
-	    ?rt2("check and maybe incr - incr", [Direction, Val, Limit]),
+	    ?rt2("check and maybe incr - incr", [{counter_value, Val}]),
 	    megaco_config:incr_pending_counter(Direction, TransId),
 	    if 
 		Val < Limit ->
 		    ok;   % Still within the limit
 		true ->
 		    ?rt2("check and maybe incr - error", 
-			 [Direction, Val, Limit]),
+			 [{counter_value, Val}]),
 		    error % Passed the limit
 	    end
     catch 
 	_:_ ->
 	    %% Has not been created yet (connect).
-	    megaco_config:cre_pending_counter(Direction, TransId, 1),
-	    ok
+	    %% Try create it, but bevare of possible raise condition
+	    try
+		begin
+		    megaco_config:cre_pending_counter(Direction, TransId, 1),
+		    ok
+		end
+	    catch
+		_:_ ->
+		    %% Ouch, raise condition, increment instead...
+		    megaco_config:incr_pending_counter(Direction, TransId),
+		    ok
+	    end
     end.
 
 
-%% check_and_maybe_incr_pending_limit(infinity, _, _) ->
-%%     ok;
-%% check_and_maybe_incr_pending_limit(Limit, Direction, TransId) ->
-%%     %% 
-%%     %% We need this kind of test to detect when we _pass_ the limit
-%%     %% 
-%%     ?rt2("check and maybe incr pending limit", [Direction, Limit, TransId]),
-%%     case (catch megaco_config:get_pending_counter(Direction, TransId)) of
-%% 	{'EXIT', _} ->
-%% 	    %% Has not been created yet (connect).
-%% 	    megaco_config:cre_pending_counter(Direction, TransId, 1),
-%% 	    ok;
-%% 	Val when Val > Limit ->
-%% 	    ?rt2("check and maybe incr - aborted", [Direction, Val, Limit]),
-%% 	    aborted;      % Already passed the limit
-%% 	Val ->
-%% 	    ?rt2("check and maybe incr - incr", [Direction, Val, Limit]),
-%% 	    megaco_config:incr_pending_counter(Direction, TransId),
-%% 	    if 
-%% 		Val < Limit ->
-%% 		    ok;   % Still within the limit
-%% 		true ->
-%% 		    ?rt2("check and maybe incr - error", 
-%% 			 [Direction, Val, Limit]),
-%% 		    error % Passed the limit
-%% 	    end
-%%     end.
-
-
 %% BUGBUG BUGBUG BUGBUG
 %% 
 %% Do we know that the Rep is still valid? A previous transaction
@@ -2648,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),
@@ -2685,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)
@@ -2716,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
 
@@ -3739,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,
@@ -4323,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},
@@ -4380,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) ->