diff options
author | Anders Svensson <[email protected]> | 2015-12-01 08:11:25 +0100 |
---|---|---|
committer | Anders Svensson <[email protected]> | 2015-12-09 13:31:22 +0100 |
commit | f8fa795ac4885af5c9f396fbcf26143a67fbdf49 (patch) | |
tree | bca6e0d7398f4f7db0a40335d89f5dae24f1fd8f /lib/diameter/src/base/diameter_traffic.erl | |
parent | c1df511623b9a2a98d4d3862ae612c1ca9837da7 (diff) | |
download | otp-f8fa795ac4885af5c9f396fbcf26143a67fbdf49.tar.gz otp-f8fa795ac4885af5c9f396fbcf26143a67fbdf49.tar.bz2 otp-f8fa795ac4885af5c9f396fbcf26143a67fbdf49.zip |
Fix request table leak at exit signal
The storing of request records in the ets table diameter_request was
wrapped in a try/after so that the latter would unconditionally remove
written entries. The problem is that it didn't deal with the process
exiting as a result of an exit signal, since this doesn't raise in an
exception. Since the process in question applies callbacks to user code,
we can potentially be linked to other process and exit as a result.
Trapping exits changes the current behaviour of the process, so spawn a
monitoring process that cleans up upon reception of 'DOWN'.
Diffstat (limited to 'lib/diameter/src/base/diameter_traffic.erl')
-rw-r--r-- | lib/diameter/src/base/diameter_traffic.erl | 32 |
1 files changed, 20 insertions, 12 deletions
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index eb4bbae931..d1adb084ce 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -1490,16 +1490,20 @@ 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. + %% 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(), + Seqs = diameter_codec:sequence_numbers(Pkt), + spawn(fun() -> diameter_lib:wait([Self]), erase_requests(Seqs) 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 @@ -1831,6 +1835,10 @@ store_request(TPid, Bin, Req, Timeout) -> TRef. %% 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), @@ -1846,8 +1854,8 @@ lookup_request(Msg, TPid) -> %% erase_requests/1 -erase_requests(Pkt) -> - ets:delete(?REQUEST_TABLE, diameter_codec:sequence_numbers(Pkt)). +erase_requests(Seqs) -> + ets:delete(?REQUEST_TABLE, Seqs). %% match_requests/1 |