{"action":"created","issue":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/744","repository_url":"https://api.github.com/repos/rabbitmq/rabbitmq-server","labels_url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/744/labels{/name}","comments_url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/744/comments","events_url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/744/events","html_url":"https://github.com/rabbitmq/rabbitmq-server/issues/744","id":148020490,"number":744,"title":"Concurrent updates to policies can trigger a `function_clause` exception","user":{"login":"dumbbell","id":159804,"avatar_url":"https://avatars.githubusercontent.com/u/159804?v=3","gravatar_id":"","url":"https://api.github.com/users/dumbbell","html_url":"https://github.com/dumbbell","followers_url":"https://api.github.com/users/dumbbell/followers","following_url":"https://api.github.com/users/dumbbell/following{/other_user}","gists_url":"https://api.github.com/users/dumbbell/gists{/gist_id}","starred_url":"https://api.github.com/users/dumbbell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dumbbell/subscriptions","organizations_url":"https://api.github.com/users/dumbbell/orgs","repos_url":"https://api.github.com/users/dumbbell/repos","events_url":"https://api.github.com/users/dumbbell/events{/privacy}","received_events_url":"https://api.github.com/users/dumbbell/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/labels/effort-low","name":"effort-low","color":"88b3e4"}],"state":"open","locked":false,"assignee":{"login":"dumbbell","id":159804,"avatar_url":"https://avatars.githubusercontent.com/u/159804?v=3","gravatar_id":"","url":"https://api.github.com/users/dumbbell","html_url":"https://github.com/dumbbell","followers_url":"https://api.github.com/users/dumbbell/followers","following_url":"https://api.github.com/users/dumbbell/following{/other_user}","gists_url":"https://api.github.com/users/dumbbell/gists{/gist_id}","starred_url":"https://api.github.com/users/dumbbell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dumbbell/subscriptions","organizations_url":"https://api.github.com/users/dumbbell/orgs","repos_url":"https://api.github.com/users/dumbbell/repos","events_url":"https://api.github.com/users/dumbbell/events{/privacy}","received_events_url":"https://api.github.com/users/dumbbell/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2016-04-13T10:48:47Z","updated_at":"2016-04-13T11:02:46Z","closed_at":null,"body":"Discovered while working on #725, concurrent executions of `rabbitmqctl set_policy` my cause the following exception:\r\n\r\n```\r\n{error,\r\n {function_clause,\r\n  [{rabbit_policy,'-match/2-lc$^0/1-0-',\r\n    [{'EXIT',\r\n      {aborted,\r\n       {cyclic,'rmq-ct-unit_inbroker_SUITE-21000@localhost',\r\n        {rabbit_runtime_parameters,'______WHOLETABLE_____'},\r\n        read,read,       \r\n        {tid,145,<9838.1856.0>}}}}],\r\n    [{file,\"src/rabbit_policy.erl\"},{line,292}]},\r\n   {rabbit_policy,match,2,\r\n    [{file,\"src/rabbit_policy.erl\"},{line,292}]},\r\n   {rabbit_policy,update_exchange,2,\r\n    [{file,\"src/rabbit_policy.erl\"},{line,259}]},\r\n   {rabbit_policy,'-update_policies/1-lc$^2/1-1-',2,\r\n    [{file,\"src/rabbit_policy.erl\"},{line,248}]},\r\n   {rabbit_policy,'-update_policies/1-fun-0-',1,\r\n    [{file,\"src/rabbit_policy.erl\"},{line,248}]},\r\n   {mnesia_tm,apply_fun,3,\r\n    [{file,\"mnesia_tm.erl\"},{line,833}]},\r\n   {mnesia_tm,execute_transaction,5,\r\n    [{file,\"mnesia_tm.erl\"},{line,808}]},\r\n   {rabbit_misc,'-execute_mnesia_transaction/1-fun-0-',1,\r\n    [{file,\"src/rabbit_misc.erl\"},{line,542}]}]}}\r\n```\r\n\r\nThe problem comes from this code in `rabbit_policy.erl`:\r\n\r\n```erlang\r\n    {Xs, Qs} = rabbit_misc:execute_mnesia_transaction(\r\n                 fun() ->\r\n                         [mnesia:lock({table, T}, write) || T <- Tabs], %% [1]\r\n                         case catch list(VHost) of\r\n                             {error, {no_such_vhost, _}} ->\r\n                                 ok; %% [2]\r\n                             Policies ->\r\n                                 {[update_exchange(X, Policies) ||\r\n                                      X <- rabbit_exchange:list(VHost)],\r\n                                  [update_queue(Q, Policies) ||\r\n                                      Q <- rabbit_amqqueue:list(VHost)]}\r\n                         end\r\n                 end),\r\n```\r\n\r\n`list(VHost)` calls `rabbit_runtime_parameters:list(VHost), <<\"policy\">>)`, which calls `rabbit_misc:dirty_read_all()`, which finally calls `mnesia:dirty_select()`.\r\n\r\nSo sometimes, `mnesia:dirty_select()` throws a \"transaction abort\" exception. This exception is caught because we use `catch list(VHost)`. Therefore, we call `update_exchange(X, Exception)` and `update_queue(Q, Exception)`. This causes the `function_clause` above.\r\n\r\nI don't see the point of catching this Mnesia abort if the code crashes later with an obscure unrelated error. I removed this `catch` and the problem never occurred again and the testsuite is green. I guess Mnesia retries the whole transaction which eventually succeeds."},"comment":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/comments/209368178","html_url":"https://github.com/rabbitmq/rabbitmq-server/issues/744#issuecomment-209368178","issue_url":"https://api.github.com/repos/rabbitmq/rabbitmq-server/issues/744","id":209368178,"user":{"login":"dumbbell","id":159804,"avatar_url":"https://avatars.githubusercontent.com/u/159804?v=3","gravatar_id":"","url":"https://api.github.com/users/dumbbell","html_url":"https://github.com/dumbbell","followers_url":"https://api.github.com/users/dumbbell/followers","following_url":"https://api.github.com/users/dumbbell/following{/other_user}","gists_url":"https://api.github.com/users/dumbbell/gists{/gist_id}","starred_url":"https://api.github.com/users/dumbbell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dumbbell/subscriptions","organizations_url":"https://api.github.com/users/dumbbell/orgs","repos_url":"https://api.github.com/users/dumbbell/repos","events_url":"https://api.github.com/users/dumbbell/events{/privacy}","received_events_url":"https://api.github.com/users/dumbbell/received_events","type":"User","site_admin":false},"created_at":"2016-04-13T11:02:46Z","updated_at":"2016-04-13T11:02:46Z","body":"This `catch` was introduced in 44e1aca07b15215dd90ec44f78007e10706e97cf. The goal is to catch `throw({error, {no_such_vhost, VHost}}`. So just removing this `catch` isn't enough."}}