aboutsummaryrefslogtreecommitdiffstats
path: root/lib/diameter
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2015-12-09 15:11:52 +0100
committerAnders Svensson <[email protected]>2015-12-09 15:11:52 +0100
commit0dc640a73faba6e3f00d45e6b2739ce055d977cd (patch)
tree0b83b3e9c8ace3251b21ba83ad4c62fa99a7a009 /lib/diameter
parent3e0e79218065458b816cd726bd637f1ecd00a882 (diff)
parent6c9cbd96d01da3194715d3caf8aa23350dfaa53a (diff)
downloadotp-0dc640a73faba6e3f00d45e6b2739ce055d977cd.tar.gz
otp-0dc640a73faba6e3f00d45e6b2739ce055d977cd.tar.bz2
otp-0dc640a73faba6e3f00d45e6b2739ce055d977cd.zip
Merge branch 'anders/diameter/request_leak/OTP-13137' into maint
* anders/diameter/request_leak/OTP-13137: Fix request table leak at retransmission Fix request table leak at exit signal
Diffstat (limited to 'lib/diameter')
-rw-r--r--lib/diameter/src/base/diameter_traffic.erl77
1 files changed, 38 insertions, 39 deletions
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 9e14860693..07f39c562f 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -169,7 +169,7 @@ incr_error(Dir, Id, TPid, _) ->
incr_error(Dir, Id, TPid) ->
incr(TPid, {Id, Dir, error}).
-
+
%% ---------------------------------------------------------------------------
%% incr_rc/4
%% ---------------------------------------------------------------------------
@@ -1485,16 +1485,12 @@ send_R(Pkt0,
caps = Caps,
packet = Pkt0},
- try
- incr(send, Pkt, TPid, AppDict),
- TRef = send_request(TPid, Pkt, Req, SvcName, Timeout),
- Pid ! Ref, %% tell caller a send has been attempted
- handle_answer(SvcName,
- App,
- recv_A(Timeout, SvcName, App, Opts, {TRef, Req}))
- after
- erase_requests(Pkt)
- end.
+ incr(send, Pkt, TPid, AppDict),
+ TRef = send_request(TPid, Pkt, Req, SvcName, Timeout),
+ Pid ! Ref, %% tell caller a send has been attempted
+ handle_answer(SvcName,
+ App,
+ recv_A(Timeout, SvcName, App, Opts, {TRef, Req})).
%% recv_A/5
@@ -1694,9 +1690,18 @@ encode(_, _, #diameter_packet{} = Pkt) ->
send_request(TPid, #diameter_packet{bin = Bin} = Pkt, Req, _SvcName, Timeout)
when node() == node(TPid) ->
- %% Store the outgoing request before sending to avoid a race with
- %% reply reception.
- TRef = store_request(TPid, Bin, Req, Timeout),
+ Seqs = diameter_codec:sequence_numbers(Bin),
+ TRef = erlang:start_timer(Timeout, self(), TPid),
+ Entry = {Seqs, Req, TRef},
+
+ %% Ensure that request table is cleaned even if we receive an exit
+ %% signal. An alternative would be to simply trap exits, but
+ %% callbacks are applied in this process, and these could possibly
+ %% be expecting the prevailing behaviour.
+ Self = self(),
+ spawn(fun() -> diameter_lib:wait([Self]), erase_request(Entry) end),
+
+ store_request(Entry, TPid),
send(TPid, Pkt),
TRef;
@@ -1711,31 +1716,21 @@ send_request(TPid, #diameter_packet{} = Pkt, Req, SvcName, Timeout) ->
%% send/1
send({TPid, Pkt, #request{handler = Pid} = Req0, SvcName, Timeout, TRef}) ->
- Seqs = diameter_codec:sequence_numbers(Pkt),
Req = Req0#request{handler = self()},
- Ref = send_request(TPid, Pkt, Req, SvcName, Timeout),
-
- try
- recv(TPid, Pid, TRef, Ref)
- after
- %% Remove only the entry for this specific send since a resend
- %% from the originating node can pick another transport on
- %% this one.
- ets:delete_object(?REQUEST_TABLE, {Seqs, Req, Ref})
- end.
+ recv(TPid, Pid, TRef, send_request(TPid, Pkt, Req, SvcName, Timeout)).
%% recv/4
%%
%% Relay an answer from a remote node.
-recv(TPid, Pid, TRef, Ref) ->
+recv(TPid, Pid, TRef, LocalTRef) ->
receive
{answer, _, _, _, _} = A ->
Pid ! A;
- {failover = T, Ref} ->
+ {failover = T, LocalTRef} ->
Pid ! {T, TRef};
T ->
- exit({timeout, Ref, TPid} = T)
+ exit({timeout, LocalTRef, TPid} = T)
end.
%% send/2
@@ -1812,17 +1807,21 @@ resend_request(Pkt0,
TRef = send_request(TPid, Pkt, Req, SvcName, Tmo),
{TRef, Req}.
-%% store_request/4
+%% store_request/2
-store_request(TPid, Bin, Req, Timeout) ->
- Seqs = diameter_codec:sequence_numbers(Bin),
- TRef = erlang:start_timer(Timeout, self(), TPid),
- ets:insert(?REQUEST_TABLE, {Seqs, Req, TRef}),
+store_request(T, TPid) ->
+ ets:insert(?REQUEST_TABLE, T),
ets:member(?REQUEST_TABLE, TPid)
- orelse (self() ! {failover, TRef}), %% failover/1 may have missed
- TRef.
+ orelse begin
+ {_Seqs, _Req, TRef} = T,
+ (self() ! {failover, TRef}) %% failover/1 may have missed
+ end.
%% lookup_request/2
+%%
+%% Note the match on both the key and transport pid. The latter is
+%% necessary since the same Hop-by-Hop and End-to-End identifiers are
+%% reused in the case of retransmission.
lookup_request(Msg, TPid) ->
Seqs = diameter_codec:sequence_numbers(Msg),
@@ -1836,10 +1835,10 @@ lookup_request(Msg, TPid) ->
false
end.
-%% erase_requests/1
+%% erase_request/1
-erase_requests(Pkt) ->
- ets:delete(?REQUEST_TABLE, diameter_codec:sequence_numbers(Pkt)).
+erase_request(T) ->
+ ets:delete_object(?REQUEST_TABLE, T).
%% match_requests/1
@@ -1862,7 +1861,7 @@ failover(TPid)
when is_pid(TPid) ->
lists:foreach(fun failover/1, match_requests(TPid));
%% Note that a request process can store its request after failover
-%% notifications are sent here: store_request/4 sends the notification
+%% notifications are sent here: store_request/2 sends the notification
%% in that case.
%% Failover as a consequence of request_peer_down/1: inform the