Skip to content

fix: give natively accepted sockets ids that can never collide with JS socket ids - #247

Open
MrGhost23 wants to merge 1 commit into
Rapsssito:masterfrom
MrGhost23:fix/native-socket-id-collision
Open

MrGhost23 wants to merge 1 commit into
Rapsssito:masterfrom
MrGhost23:fix/native-socket-id-collision

Conversation

@MrGhost23

Copy link
Copy Markdown

Summary

Fixes #246: a JS-created socket can reuse the id of a live accepted socket. The native connect()/listen() then fails for that id, and the error destroys the live accepted connection.

JS numbers sockets from 0 (src/Globals.js), while natively accepted sockets are numbered 5000 + n on iOS and (1 + serverId) * 1000 + n on Android. Nothing keeps the ranges apart, so once an app has created enough sockets in one session, they collide.

Changes

Natively accepted sockets now get negative ids (-1, -2, …) from a single counter per platform:

  • iOS (TcpSockets.m): getNextId returns -(++_counter). The COUNTER_OFFSET define is removed.
  • Android (TcpSocketServer.java): replaced the per-server (1 + serverId) * 1000 range with one shared AtomicInteger, returning -incrementAndGet().

JS ids start at 0 and only grow, so they can never be negative and the two id spaces can't overlap. On Android, two servers also stop sharing overlapping ranges. Using an AtomicInteger keeps id assignment safe across the listen threads.

Compatibility

  • Ids are internal (Socket._id) and only compared for equality, never ordered or range-checked. I checked src/, ios/ and android/.
  • Accepted-socket ids were never documented, so this is a fix, not a breaking change.
  • iOS uses the id as the GCDAsyncSocket read tag (long), which accepts negative values.

Notes

Test plan

Closes #246

…S socket ids

JS numbers every Socket and Server it creates from a counter starting at 0,
while the native side numbers the sockets a server accepts from its own
counter (iOS: 5000 + n, Android: (1 + serverId) * 1000 + n). Nothing keeps
the two ranges apart, and Server._buildSocket consumes a JS id for every
accepted connection, so a long-running app eventually creates a JS socket
whose id is already used by a live accepted connection.

When that happens, connect()/listen() on the reused id fails with "this
client's socket is already connected" (iOS) or "connect() called twice with
the same id." (Android). The error is emitted for that id, so both JS sockets
receive it, and the Socket error handler destroys the live accepted
connection.

Accepted sockets now get negative ids (-1, -2, ...) from a single counter on
each platform. JS ids are never negative, so the two ranges cannot overlap,
and on Android two servers can no longer hand out overlapping ranges.
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.

Socket id collision: a new JS socket can reuse a live accepted socket's id, which then gets destroyed

1 participant