From 16d2e129ff59db4917b0495d914df646f62669b9 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 2 Jun 2010 23:31:23 +0000 Subject: OTP-8634: Pending counter raise condition. --- lib/megaco/doc/src/megaco.xml | 16 +++++-- lib/megaco/doc/src/notes.xml | 7 +++ lib/megaco/src/app/megaco_internal.hrl | 26 ++++++++-- lib/megaco/src/engine/megaco_config.erl | 26 +++------- lib/megaco/src/engine/megaco_messenger.erl | 77 +++++++----------------------- lib/megaco/vsn.mk | 4 +- 6 files changed, 68 insertions(+), 88 deletions(-) (limited to 'lib') diff --git a/lib/megaco/doc/src/megaco.xml b/lib/megaco/doc/src/megaco.xml index 0fb9d5aac6..ae9e250965 100644 --- a/lib/megaco/doc/src/megaco.xml +++ b/lib/megaco/doc/src/megaco.xml @@ -4,7 +4,7 @@
- 20002009 + 20002010 Ericsson AB. All Rights Reserved. @@ -13,12 +13,12 @@ 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. - + megaco @@ -40,6 +40,16 @@
DATA TYPES Own Id: OTP-8627

+ +

Eliminated a possible raise condition while creating + pending counters.

+

Own Id: OTP-8634

+

Aux Id: Seq 11579

+ +
diff --git a/lib/megaco/src/app/megaco_internal.hrl b/lib/megaco/src/app/megaco_internal.hrl index adbaacacef..2c124e9060 100644 --- a/lib/megaco/src/app/megaco_internal.hrl +++ b/lib/megaco/src/app/megaco_internal.hrl @@ -1,19 +1,19 @@ %% %% %CopyrightBegin% -%% -%% Copyright Ericsson AB 1999-2009. All Rights Reserved. -%% +%% +%% Copyright Ericsson AB 1999-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% %% @@ -139,6 +139,22 @@ [?APPLICATION, ?MODULE, self()|A]))). +-define(megaco_ereport(Label, Report), + ?megaco_report(error_report, Label, Report)). + +-define(megaco_wreport(Label, Report), + ?megaco_report(warning_report, Label, Report)). + +-define(megaco_ireport(Label, Report), + ?megaco_report(info_report, Label, Report)). + +-define(megaco_report(Func, Label, Report), + (catch error_logger:Func([{label, Label}, + {application, ?APPLICATION}, + {module, ?MODULE}, + {process, self()} | Report]))). + + %%%---------------------------------------------------------------------- %%% Default (ignore) value of the Extra argument to the %%% megaco:receive_message/5 and process_received_message functions/5. diff --git a/lib/megaco/src/engine/megaco_config.erl b/lib/megaco/src/engine/megaco_config.erl index 0445f10838..6805db790d 100644 --- a/lib/megaco/src/engine/megaco_config.erl +++ b/lib/megaco/src/engine/megaco_config.erl @@ -628,31 +628,19 @@ incr_counter(Item, Incr) -> end catch error:_ -> + %% Counter does not exist, so try creat it try begin cre_counter(Item, Incr) end catch exit:_ -> - %% Ok, some other process got there before us, - %% so try again + %% This is a raise condition. + %% When we tried to update the counter above, it + %% did not exist, but now it does... ets:update_counter(megaco_config, Item, Incr) end end. -%% incr_counter(Item, Incr) -> -%% case (catch ets:update_counter(megaco_config, Item, Incr)) of -%% {'EXIT', _} -> -%% case (catch cre_counter(Item, Incr)) of -%% {'EXIT', _} -> -%% %% Ok, some other process got there before us, -%% %% so try again -%% ets:update_counter(megaco_config, Item, Incr); -%% NewVal -> -%% NewVal -%% end; -%% NewVal -> -%% NewVal -%% end. cre_counter(Item, Initial) -> case whereis(?SERVER) =:= self() of @@ -660,8 +648,8 @@ cre_counter(Item, Initial) -> case call({cre_counter, Item, Initial}) of {ok, Value} -> Value; - Error -> - exit(Error) + {error, Reason} -> + exit({failed_creating_counter, Item, Initial, Reason}) end; true -> %% Check that the counter does not already exists @@ -671,7 +659,7 @@ cre_counter(Item, Initial) -> ets:insert(megaco_config, {Item, Initial}), {ok, Initial}; [_] -> - %% Ouch, now what? + %% Possibly a raise condition {error, already_exists} end diff --git a/lib/megaco/src/engine/megaco_messenger.erl b/lib/megaco/src/engine/megaco_messenger.erl index 5756e8e896..d6e0918a31 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 diff --git a/lib/megaco/vsn.mk b/lib/megaco/vsn.mk index cf5957460d..9904824382 100644 --- a/lib/megaco/vsn.mk +++ b/lib/megaco/vsn.mk @@ -19,10 +19,10 @@ APPLICATION = megaco MEGACO_VSN = 3.14.1 -PRE_VSN = +PRE_VSN =-p02 APP_VSN = "$(APPLICATION)-$(MEGACO_VSN)$(PRE_VSN)" -TICKETS = OTP-8561 OTP-8627 +TICKETS = OTP-8561 OTP-8627 OTP-8634 TICKETS_3_14 = OTP-8317 OTP-8323 OTP-8328 OTP-8362 OTP-8403 -- cgit v1.2.3