search

crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.47k stars 763 forks source link

Check first and then do increment #1636

Closed SunFulong closed 2 months ago

SunFulong commented 2 months ago

Wrapping around at the boundary should do check first.

oberstet commented 2 months ago

This is wrong. It won't create an ID 9007199254740992. It is also unnecessary, the code as it stands is correct.

the bug I mentioned in the other thread is on this line:

https://github.com/crossbario/autobahn-python/blob/924604ed250b565bcd17a30c101a86c67864811e/autobahn/util.py#L322

The 0 must be 1 ...

SunFulong commented 2 months ago

It won't create an ID 9007199254740992.

Although it won't created, but it can be reached at previous round.

oberstet commented 2 months ago

Although it won't created, but it can be reached at previous round.

yes, you are right actually, it would spit out 9007199254740992. however, I stand by my other comment: it is unnecessary (IOW: the PR code does not change the functional behavior in any way. and non-functional behavior .. performance .. I am not convinced it would be an improvement ... or even necessary anyways.

if you file a PR that changes the 0 to 1 to fix the bug I mentioned above, that would be highly welcome! I'd merged it right away ...

SunFulong commented 2 months ago

if you file a PR that changes the 0 to 1 to fix the bug

See #1637