{"action":"created","issue":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/120","labels_url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/120/labels{/name}","comments_url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/120/comments","events_url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/120/events","html_url":"https://github.com/rabbitmq/rabbitmq-java-client/issues/120","id":123993974,"number":120,"title":"channel.queueDelete sometimes throws NullPointerException, possibly due to race on recordedBindings","user":{"login":"spatula75","id":7718879,"avatar_url":"https://avatars.githubusercontent.com/u/7718879?v=3","gravatar_id":"","url":"https://api.github.com/users/spatula75","html_url":"https://github.com/spatula75","followers_url":"https://api.github.com/users/spatula75/followers","following_url":"https://api.github.com/users/spatula75/following{/other_user}","gists_url":"https://api.github.com/users/spatula75/gists{/gist_id}","starred_url":"https://api.github.com/users/spatula75/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spatula75/subscriptions","organizations_url":"https://api.github.com/users/spatula75/orgs","repos_url":"https://api.github.com/users/spatula75/repos","events_url":"https://api.github.com/users/spatula75/events{/privacy}","received_events_url":"https://api.github.com/users/spatula75/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/labels/effort-low","name":"effort-low","color":"88b3e4"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/milestones/11","html_url":"https://github.com/rabbitmq/rabbitmq-java-client/milestones/3.6.1","labels_url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/milestones/11/labels","id":1441747,"number":11,"title":"3.6.1","description":null,"creator":{"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},"open_issues":1,"closed_issues":0,"state":"open","created_at":"2015-12-03T10:56:38Z","updated_at":"2015-12-30T00:44:54Z","due_on":null,"closed_at":null},"comments":11,"created_at":"2015-12-27T19:46:48Z","updated_at":"2015-12-30T00:59:45Z","closed_at":null,"body":"When multiple channels share an AutorecoveringConnection and queues are coming and going from multiple theads, about 1 time out of 10000 , we are seeing a NullPointerException or ConcurrentModificationException to be thrown:\r\n\r\n```\r\njava.lang.NullPointerException\r\n        at com.rabbitmq.client.impl.recovery.AutorecoveringConnection.removeBindingsWithDestination(AutorecoveringConnection.java:768)\r\n        at com.rabbitmq.client.impl.recovery.AutorecoveringConnection.deleteRecordedQueue(AutorecoveringConnection.java:690)\r\n        at com.rabbitmq.client.impl.recovery.AutorecoveringChannel.deleteRecordedQueue(AutorecoveringChannel.java:562)\r\n        at com.rabbitmq.client.impl.recovery.AutorecoveringChannel.queueDelete(AutorecoveringChannel.java:303)\r\n        at com.rabbitmq.client.impl.recovery.AutorecoveringChannel.queueDelete(AutorecoveringChannel.java:299)\r\n```\r\n\r\nThe code at this location looks innocuous enough:\r\n```\r\n    Set<RecordedBinding> removeBindingsWithDestination(String s) {\r\n        Set<RecordedBinding> result = new HashSet<RecordedBinding>();\r\n        for (Iterator<RecordedBinding> it = this.recordedBindings.iterator(); it.hasNext(); ) {\r\n            RecordedBinding b = it.next();\r\n            if(b.getDestination().equals(s)) { // [[ NPE here ]] //\r\n                it.remove();\r\n                result.add(b);\r\n            }\r\n        }\r\n        return result;\r\n    }\r\n```\r\n\r\nHowever, `removeBindingsWithDestination` is not synchronized, so it's possible for multiple threads to be in this method changing the recordedBindings ArrayList at the same time as bindings come and go from multiple threads, even with channels not being shared across threads (but the Connection is shared).\r\n\r\nIt looks the other accesses to recordedBindings are synchronized (though caution: `maybeDeleteRecordedAutoDeleteExchange` is synchronizing differently than the others) so `removeBindingsWithDestination` may have just fallen through the cracks.\r\n\r\nIt may be a sufficient fix just to make `removeBindingsWithDestination` synchronized.\r\n\r\n(Another tangential thing I noticed is that most of the recorded maintenance is done from synchronized methods, but `maybeDeleteRecordedAutoDeleteQueue` during a `basicCancel` instead synchronizes on `recordedQueues` and `consumers`, and that this method just removes the `recordedQueues` entry, not the `recordedBindings` associated with the queue as well.)"},"comment":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/comments/167912459","html_url":"https://github.com/rabbitmq/rabbitmq-java-client/issues/120#issuecomment-167912459","issue_url":"https://api.github.com/repos/rabbitmq/rabbitmq-java-client/issues/120","id":167912459,"user":{"login":"spatula75","id":7718879,"avatar_url":"https://avatars.githubusercontent.com/u/7718879?v=3","gravatar_id":"","url":"https://api.github.com/users/spatula75","html_url":"https://github.com/spatula75","followers_url":"https://api.github.com/users/spatula75/followers","following_url":"https://api.github.com/users/spatula75/following{/other_user}","gists_url":"https://api.github.com/users/spatula75/gists{/gist_id}","starred_url":"https://api.github.com/users/spatula75/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spatula75/subscriptions","organizations_url":"https://api.github.com/users/spatula75/orgs","repos_url":"https://api.github.com/users/spatula75/repos","events_url":"https://api.github.com/users/spatula75/events{/privacy}","received_events_url":"https://api.github.com/users/spatula75/received_events","type":"User","site_admin":false},"created_at":"2015-12-30T00:59:45Z","updated_at":"2015-12-30T00:59:45Z","body":"Pardon my ignorance; what's the preferred method of submitting a PR?  I haven't had a need of doing such a thing yet."}}