{"action":"created","issue":{"url":"https://api.github.com/repos/inre/rust-mq/issues/14","repository_url":"https://api.github.com/repos/inre/rust-mq","labels_url":"https://api.github.com/repos/inre/rust-mq/issues/14/labels{/name}","comments_url":"https://api.github.com/repos/inre/rust-mq/issues/14/comments","events_url":"https://api.github.com/repos/inre/rust-mq/issues/14/events","html_url":"https://github.com/inre/rust-mq/issues/14","id":173986926,"number":14,"title":"Trouble with Packet Identifiers","user":{"login":"hutch","id":4345,"avatar_url":"https://avatars.githubusercontent.com/u/4345?v=3","gravatar_id":"","url":"https://api.github.com/users/hutch","html_url":"https://github.com/hutch","followers_url":"https://api.github.com/users/hutch/followers","following_url":"https://api.github.com/users/hutch/following{/other_user}","gists_url":"https://api.github.com/users/hutch/gists{/gist_id}","starred_url":"https://api.github.com/users/hutch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hutch/subscriptions","organizations_url":"https://api.github.com/users/hutch/orgs","repos_url":"https://api.github.com/users/hutch/repos","events_url":"https://api.github.com/users/hutch/events{/privacy}","received_events_url":"https://api.github.com/users/hutch/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-08-30T10:44:41Z","updated_at":"2016-08-30T11:34:41Z","closed_at":null,"body":"Background: I posted a question to stack overflow: http://stackoverflow.com/questions/39210834/do-any-mqtt-clients-actually-reuse-packet-identifiers\r\n\r\nThe packet identifier is required for certain MQTT control packets (http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/csprd02/mqtt-v3.1.1-csprd02.html#_Toc385349761). It's defined by the standard as a 16bit integer (presumably), and is generated by each client. The identifiers are reusable by the client after the acknowledgement packet is received. So the standard allows up to 64k in-flight messages. \r\n\r\nrust-mq/mqtt3 implements the packet identifier as a u16, and we have this implementation:\r\n\r\n```rust\r\n#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]\r\npub struct PacketIdentifier(pub u16);\r\n\r\nimpl PacketIdentifier {\r\n    pub fn zero() -> PacketIdentifier {\r\n        PacketIdentifier(0)\r\n    }\r\n\r\n    pub fn next(&self) -> PacketIdentifier {\r\n        PacketIdentifier(self.0 + 1)\r\n    }\r\n}\r\n```\r\n\r\nProblem 1: `next` can be made to overflow and panic if more than 64k messages requiring a packet identifier is sent by a single client. (My very first test client panicked :-)\r\n\r\nProblem 2: even if it didn't panic, each client would still be limited to a *total* of 64k messages requiring a packet identifier.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/inre/rust-mq/issues/comments/243411526","html_url":"https://github.com/inre/rust-mq/issues/14#issuecomment-243411526","issue_url":"https://api.github.com/repos/inre/rust-mq/issues/14","id":243411526,"user":{"login":"kteza1","id":6826529,"avatar_url":"https://avatars.githubusercontent.com/u/6826529?v=3","gravatar_id":"","url":"https://api.github.com/users/kteza1","html_url":"https://github.com/kteza1","followers_url":"https://api.github.com/users/kteza1/followers","following_url":"https://api.github.com/users/kteza1/following{/other_user}","gists_url":"https://api.github.com/users/kteza1/gists{/gist_id}","starred_url":"https://api.github.com/users/kteza1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kteza1/subscriptions","organizations_url":"https://api.github.com/users/kteza1/orgs","repos_url":"https://api.github.com/users/kteza1/repos","events_url":"https://api.github.com/users/kteza1/events{/privacy}","received_events_url":"https://api.github.com/users/kteza1/received_events","type":"User","site_admin":false},"created_at":"2016-08-30T11:34:41Z","updated_at":"2016-08-30T11:34:41Z","body":"@hutch \r\n\r\nProblem 1:  Did you test in release mode? As far as I know, overflows are panics in debug mode but 2's complement in release mode. But I agree that we shouldn't let it overflow even in debug mode (Which I'm planning to do in my client and test. So thanks for raising this :) )\r\n\r\nProblem 2: I don't think reusing a pkid is a problem. Usually outgoing publishes in the queue can be bounded (with a limit not > u16) which is what I'm doing [here](https://github.com/Ather-Energy/RuMqtt/blob/master/src/clientoptions.rs#L16). But yeah, an outgoing publish queue with messages containing same pkids might be problem if acks are not returned in sequence (say one of the acks got lost and wrong message being popped out from the queue)"}}
{"actor":{"display_login":"kteza1"}}