Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #585 MessageId not limited to 65535 #586

Conversation

hylkevds
Copy link
Collaborator

Message ID should always be in the range 1 - 65535. This adds a test, and resets the counter to 1 when needed.

@hylkevds hylkevds force-pushed the fix_585_MessageId_not_limited_to_65535 branch from 27e41c4 to 3692323 Compare May 9, 2021 16:36
Copy link
Collaborator

@andsel andsel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could avoid the synchronized and leverage the updateAndGet insteaed of `incrementAndGet '

@@ -503,7 +503,22 @@ public void resendNotAckedPublishes() {
}

int nextPacketId() {
return lastPacketId.incrementAndGet();
final int next = lastPacketId.incrementAndGet();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify with

return lastPacketId.updateAndGet(v -> v == 65_536 ? 1 : v + 1);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good point.
Much nicer :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should compare to 65_535, since 65_536 is already invalid...

Message ID should always be in the range 1 - 65535. This adds a test,
and, and resets the counter to 1 when needed.
@hylkevds hylkevds force-pushed the fix_585_MessageId_not_limited_to_65535 branch from 3692323 to ef89026 Compare July 2, 2021 13:41
@hylkevds
Copy link
Collaborator Author

hylkevds commented Jul 2, 2021

Simplified now :)

@andsel andsel merged commit 06c64bf into moquette-io:master Jul 2, 2021
@hylkevds hylkevds deleted the fix_585_MessageId_not_limited_to_65535 branch July 2, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants