mirror of
https://github.com/processone/ejabberd.git
synced 2024-11-20 16:15:59 +01:00
Prevent race condition when calling get_caps while note_caps has not been handled yet (EJAB-934)
SVN Revision: 2071
This commit is contained in:
Christophe Romain
parent
077b9cd4db
commit
20088d8a92
@ -1,3 +1,9 @@
|
||||
2009-05-11 Christophe Romain <christophe.romain@process-one.net>
|
||||
|
||||
* src/mod_caps.erl: Prevent race condition when calling get_caps while
|
||||
note_caps has not been handled yet (EJAB-934)
|
||||
* src/mod_pubsub/mod_pubsub.erl: Likewise
|
||||
|
||||
2009-05-08 Christophe Romain <christophe.romain@process-one.net>
|
||||
|
||||
* src/mod_pubsub/mod_pubsub.erl: Allow to get subscriptions on a given
|
||||
|
||||
@ -35,6 +35,7 @@
|
||||
-export([read_caps/1,
|
||||
get_caps/1,
|
||||
note_caps/3,
|
||||
wait_caps/2,
|
||||
clear_caps/1,
|
||||
get_features/2,
|
||||
get_user_resources/2,
|
||||
@ -55,7 +56,8 @@
|
||||
|
||||
%% hook handlers
|
||||
-export([receive_packet/3,
|
||||
receive_packet/4]).
|
||||
receive_packet/4,
|
||||
presence_probe/3]).
|
||||
|
||||
-include("ejabberd.hrl").
|
||||
-include("jlib.hrl").
|
||||
@ -101,11 +103,24 @@ read_caps([], Result) ->
|
||||
Result.
|
||||
|
||||
%% get_caps reads user caps from database
|
||||
get_caps(JID) ->
|
||||
case catch mnesia:dirty_read({user_caps, jid_to_binary(JID)}) of
|
||||
[#user_caps{caps=Caps}] ->
|
||||
%% here we handle a simple retry loop, to avoid race condition
|
||||
%% when asking caps while we still did not called note_caps
|
||||
%% timeout is set to 10s
|
||||
%% this is to be improved, but without altering performances.
|
||||
%% if we did not get user presence 10s after getting presence_probe
|
||||
%% we assume it has no caps
|
||||
get_caps(LJID) ->
|
||||
get_caps(LJID, 5).
|
||||
get_caps(_, 0) ->
|
||||
nothing;
|
||||
get_caps(LJID, Retry) ->
|
||||
case catch mnesia:dirty_read({user_caps, jid_to_binary(LJID)}) of
|
||||
[#user_caps{caps=waiting}] ->
|
||||
timer:sleep(2000),
|
||||
get_caps(LJID, Retry-1);
|
||||
[#user_caps{caps=Caps}] ->
|
||||
Caps;
|
||||
_ ->
|
||||
_ ->
|
||||
nothing
|
||||
end.
|
||||
|
||||
@ -139,6 +154,13 @@ note_caps(Host, From, Caps) ->
|
||||
gen_server:cast(Proc, {note_caps, From, Caps})
|
||||
end.
|
||||
|
||||
%% wait_caps should be called just before note_caps
|
||||
%% it allows to lock get_caps usage for code using presence_probe
|
||||
%% that may run before we get any chance to note_caps.
|
||||
wait_caps(Host, From) ->
|
||||
Proc = gen_mod:get_module_proc(Host, ?PROCNAME),
|
||||
gen_server:cast(Proc, {wait_caps, From}).
|
||||
|
||||
%% get_features returns a list of features implied by the given caps
|
||||
%% record (as extracted by read_caps). It may block, and may signal a
|
||||
%% timeout error.
|
||||
@ -197,6 +219,9 @@ receive_packet(_, _, _) ->
|
||||
receive_packet(_JID, From, To, Packet) ->
|
||||
receive_packet(From, To, Packet).
|
||||
|
||||
presence_probe(From, To, _) ->
|
||||
wait_caps(To#jid.lserver, From).
|
||||
|
||||
jid_to_binary(JID) ->
|
||||
list_to_binary(jlib:jid_to_string(JID)).
|
||||
|
||||
@ -226,10 +251,11 @@ init([Host, _Opts]) ->
|
||||
{type, bag},
|
||||
{attributes, record_info(fields, user_caps_resources)}]),
|
||||
mnesia:delete_table(user_caps_default),
|
||||
mnesia:clear_table(user_caps),
|
||||
mnesia:clear_table(user_caps_resources),
|
||||
mnesia:clear_table(user_caps), % clean in case of explicitely set to disc_copies
|
||||
mnesia:clear_table(user_caps_resources), % clean in case of explicitely set to disc_copies
|
||||
ejabberd_hooks:add(user_receive_packet, Host, ?MODULE, receive_packet, 30),
|
||||
ejabberd_hooks:add(s2s_receive_packet, Host, ?MODULE, receive_packet, 30),
|
||||
ejabberd_hooks:add(presence_probe_hook, Host, ?MODULE, presence_probe, 20),
|
||||
{ok, #state{host = Host}}.
|
||||
|
||||
maybe_get_features(#caps{node = Node, version = Version, exts = Exts}) ->
|
||||
@ -278,15 +304,17 @@ handle_cast({note_caps, From,
|
||||
%% lots of caps disco requests.
|
||||
{U, S, R} = jlib:jid_tolower(From),
|
||||
BJID = jid_to_binary(From),
|
||||
mnesia:dirty_write(#user_caps{jid = BJID, caps = caps_to_binary(Caps)}),
|
||||
case ejabberd_sm:get_user_resources(U, S) of
|
||||
[] ->
|
||||
% only store resources of caps aware external contacts
|
||||
BUID = jid_to_binary({U, S, []}),
|
||||
mnesia:dirty_write(#user_caps_resources{uid = BUID, resource = list_to_binary(R)});
|
||||
_ ->
|
||||
ok
|
||||
end,
|
||||
mnesia:transaction(fun() ->
|
||||
mnesia:write(#user_caps{jid = BJID, caps = caps_to_binary(Caps)}),
|
||||
case ejabberd_sm:get_user_resources(U, S) of
|
||||
[] ->
|
||||
% only store resources of caps aware external contacts
|
||||
BUID = jid_to_binary({U, S, []}),
|
||||
mnesia:write(#user_caps_resources{uid = BUID, resource = list_to_binary(R)});
|
||||
_ ->
|
||||
ok
|
||||
end
|
||||
end),
|
||||
%% Now, find which of these are not already in the database.
|
||||
SubNodes = [Version | Exts],
|
||||
case lists:foldl(fun(SubNode, Acc) ->
|
||||
@ -320,6 +348,10 @@ handle_cast({note_caps, From,
|
||||
end, Requests, Missing),
|
||||
{noreply, State#state{disco_requests = NewRequests}}
|
||||
end;
|
||||
handle_cast({wait_caps, From}, State) ->
|
||||
BJID = jid_to_binary(From),
|
||||
mnesia:dirty_write(#user_caps{jid = BJID, caps = waiting}),
|
||||
{noreply, State};
|
||||
handle_cast({disco_response, From, _To,
|
||||
#iq{type = Type, id = ID,
|
||||
sub_el = SubEls}},
|
||||
@ -394,6 +426,7 @@ terminate(_Reason, State) ->
|
||||
Host = State#state.host,
|
||||
ejabberd_hooks:delete(user_receive_packet, Host, ?MODULE, receive_packet, 30),
|
||||
ejabberd_hooks:delete(s2s_receive_packet, Host, ?MODULE, receive_packet, 30),
|
||||
ejabberd_hooks:delete(presence_probe_hook, Host, ?MODULE, presence_probe, 20),
|
||||
ok.
|
||||
|
||||
code_change(_OldVsn, State, _Extra) ->
|
||||
|
||||
@ -177,7 +177,7 @@ init([ServerHost, Opts]) ->
|
||||
ejabberd_hooks:add(disco_sm_identity, ServerHost, ?MODULE, disco_sm_identity, 75),
|
||||
ejabberd_hooks:add(disco_sm_features, ServerHost, ?MODULE, disco_sm_features, 75),
|
||||
ejabberd_hooks:add(disco_sm_items, ServerHost, ?MODULE, disco_sm_items, 75),
|
||||
ejabberd_hooks:add(presence_probe_hook, ServerHost, ?MODULE, presence_probe, 50),
|
||||
ejabberd_hooks:add(presence_probe_hook, ServerHost, ?MODULE, presence_probe, 80),
|
||||
ejabberd_hooks:add(roster_in_subscription, ServerHost, ?MODULE, in_subscription, 50),
|
||||
ejabberd_hooks:add(roster_out_subscription, ServerHost, ?MODULE, out_subscription, 50),
|
||||
ejabberd_hooks:add(remove_user, ServerHost, ?MODULE, remove_user, 50),
|
||||
@ -447,54 +447,58 @@ send_loop(State) ->
|
||||
send_loop(State);
|
||||
{presence, User, Server, Resources, JID} ->
|
||||
%% get resources caps and check if processing is needed
|
||||
{HasCaps, ResourcesCaps} = lists:foldl(fun(Resource, {R, L}) ->
|
||||
case mod_caps:get_caps({User, Server, Resource}) of
|
||||
nothing -> {R, L};
|
||||
Caps -> {true, [{Resource, Caps} | L]}
|
||||
end
|
||||
end, {false, []}, Resources),
|
||||
case HasCaps of
|
||||
true ->
|
||||
Host = State#state.host,
|
||||
ServerHost = State#state.server_host,
|
||||
Owner = jlib:jid_remove_resource(jlib:jid_tolower(JID)),
|
||||
lists:foreach(fun(#pubsub_node{nodeid = {_, Node}, type = Type, id = NodeId, options = Options}) ->
|
||||
case get_option(Options, send_last_published_item) of
|
||||
on_sub_and_presence ->
|
||||
lists:foreach(fun({Resource, Caps}) ->
|
||||
CapsNotify = case catch mod_caps:get_features(ServerHost, Caps) of
|
||||
Features when is_list(Features) -> lists:member(Node ++ "+notify", Features);
|
||||
_ -> false
|
||||
end,
|
||||
case CapsNotify of
|
||||
true ->
|
||||
LJID = {User, Server, Resource},
|
||||
Subscribed = case get_option(Options, access_model) of
|
||||
open -> true;
|
||||
presence -> true;
|
||||
whitelist -> false; % subscribers are added manually
|
||||
authorize -> false; % likewise
|
||||
roster ->
|
||||
Grps = get_option(Options, roster_groups_allowed, []),
|
||||
{OU, OS, _} = Owner,
|
||||
element(2, get_roster_info(OU, OS, LJID, Grps))
|
||||
%% get_caps may be blocked few seconds, get_caps as well
|
||||
%% so we spawn the whole process not to block other queries
|
||||
spawn(fun() ->
|
||||
{HasCaps, ResourcesCaps} = lists:foldl(fun(Resource, {R, L}) ->
|
||||
case mod_caps:get_caps({User, Server, Resource}) of
|
||||
nothing -> {R, L};
|
||||
Caps -> {true, [{Resource, Caps} | L]}
|
||||
end
|
||||
end, {false, []}, Resources),
|
||||
case HasCaps of
|
||||
true ->
|
||||
Host = State#state.host,
|
||||
ServerHost = State#state.server_host,
|
||||
Owner = jlib:jid_remove_resource(jlib:jid_tolower(JID)),
|
||||
lists:foreach(fun(#pubsub_node{nodeid = {_, Node}, type = Type, id = NodeId, options = Options}) ->
|
||||
case get_option(Options, send_last_published_item) of
|
||||
on_sub_and_presence ->
|
||||
lists:foreach(fun({Resource, Caps}) ->
|
||||
CapsNotify = case catch mod_caps:get_features(ServerHost, Caps) of
|
||||
Features when is_list(Features) -> lists:member(Node ++ "+notify", Features);
|
||||
_ -> false
|
||||
end,
|
||||
if Subscribed ->
|
||||
send_items(Owner, Node, NodeId, Type, LJID, last);
|
||||
case CapsNotify of
|
||||
true ->
|
||||
LJID = {User, Server, Resource},
|
||||
Subscribed = case get_option(Options, access_model) of
|
||||
open -> true;
|
||||
presence -> true;
|
||||
whitelist -> false; % subscribers are added manually
|
||||
authorize -> false; % likewise
|
||||
roster ->
|
||||
Grps = get_option(Options, roster_groups_allowed, []),
|
||||
{OU, OS, _} = Owner,
|
||||
element(2, get_roster_info(OU, OS, LJID, Grps))
|
||||
end,
|
||||
if Subscribed ->
|
||||
send_items(Owner, Node, NodeId, Type, LJID, last);
|
||||
true ->
|
||||
ok
|
||||
end;
|
||||
false ->
|
||||
ok
|
||||
end;
|
||||
false ->
|
||||
ok
|
||||
end
|
||||
end, ResourcesCaps);
|
||||
_ ->
|
||||
ok
|
||||
end
|
||||
end, tree_action(Host, get_nodes, [Owner, JID]));
|
||||
false ->
|
||||
ok
|
||||
end,
|
||||
end
|
||||
end, ResourcesCaps);
|
||||
_ ->
|
||||
ok
|
||||
end
|
||||
end, tree_action(Host, get_nodes, [Owner, JID]));
|
||||
false ->
|
||||
ok
|
||||
end
|
||||
end),
|
||||
send_loop(State);
|
||||
stop ->
|
||||
ok
|
||||
@ -786,7 +790,7 @@ terminate(_Reason, #state{host = Host,
|
||||
ejabberd_hooks:delete(disco_sm_identity, ServerHost, ?MODULE, disco_sm_identity, 75),
|
||||
ejabberd_hooks:delete(disco_sm_features, ServerHost, ?MODULE, disco_sm_features, 75),
|
||||
ejabberd_hooks:delete(disco_sm_items, ServerHost, ?MODULE, disco_sm_items, 75),
|
||||
ejabberd_hooks:delete(presence_probe_hook, ServerHost, ?MODULE, presence_probe, 50),
|
||||
ejabberd_hooks:delete(presence_probe_hook, ServerHost, ?MODULE, presence_probe, 80),
|
||||
ejabberd_hooks:delete(roster_in_subscription, ServerHost, ?MODULE, in_subscription, 50),
|
||||
ejabberd_hooks:delete(roster_out_subscription, ServerHost, ?MODULE, out_subscription, 50),
|
||||
ejabberd_hooks:delete(remove_user, ServerHost, ?MODULE, remove_user, 50),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user