Skip to content

Conversation

phischu
Copy link
Collaborator

@phischu phischu commented Jun 23, 2025

  • Fix warnings
  • Fix bug with server_client and more than 2 clients
  • Do todos
  • High-level streaming

The web does not support TCP and for Node this API is too low level. If we wanted to create node servers eventually, we would wrap a more high-level API.

@phischu phischu force-pushed the stdlib/network branch 2 times, most recently from 0753152 to 3349353 Compare June 23, 2025 15:01
@phischu phischu marked this pull request as draft June 23, 2025 15:16
@phischu
Copy link
Collaborator Author

phischu commented Jul 1, 2025

Perhaps the other leak is the same as this one: libuv/libuv#559

@marvinborner
Copy link
Member

Looks indeed similar, I believe the 128 byte object to be some internal UV structure.

@phischu
Copy link
Collaborator Author

phischu commented Jul 30, 2025

The space leak was a stack that does not get freed when using eraseStack, but does when running it to completion. We should investigate this separately, but I do note that @topLevelEraser does not erase the stack's memory.

@phischu phischu force-pushed the stdlib/network branch 2 times, most recently from 523a4bb to ca3926a Compare July 30, 2025 09:47
@phischu phischu marked this pull request as ready for review July 30, 2025 10:27
@phischu phischu requested a review from marvinborner July 30, 2025 10:27
@phischu phischu requested a review from b-studios August 11, 2025 14:18
Comment on lines +8 to +9
/// A TCP handle. Should not be inspected.
type Connection = Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// A TCP handle. Should not be inspected.
type Connection = Int
/// A TCP handle. Should not be inspected.
extern type Connection
// = llvm "Int"

Could we make it an extern type so that it cannot be inspected?

(also applies to Listener below)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I don't like it (because then it is a Pos) but I guess I'll have to do it, because the representation in js is different.

Copy link
Contributor

@jiribenes jiribenes Aug 20, 2025

Choose a reason for hiding this comment

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

Approaching the problem from the other side: does it have to be a Pos? We could let the user choose, no?

Couldn't we fix this "properly" by some = llvm "i64" annotation there instead? 🎣

phischu and others added 14 commits September 19, 2025 16:33
This fixes a memory leak and increases the backlog of uv_listen to
SOMAXCONN, such that connections are put in a queue when the previous
acceptor is not yet finished (this is the case with a timeout, which
triggers the connects at the same time)

Another memory leak remains which I couldn't find.

```
==130847== 128 bytes in 1 blocks are definitely lost in loss record 1 of 1
==130847==    at 0x48577A8: malloc (vg_replace_malloc.c:446)
==130847==    by 0x4003F90: ??? (in server_client)
==130847==    by 0x4004499: ??? (in server_client)
==130847==    by 0x4004557: run (in server_client)
==130847==    by 0x4004A1C: spawn_4055 (in server_client)
==130847==    by 0x40044F6: resume_Int (in server_client)
==130847==    by 0x4002BA0: c_tcp_listen (io.c:367)
==130847==    by 0x4004B65: listen_4331 (in server_client)
==130847==    by 0x400CA5D: effektMain (in server_client)
==130847==    by 0x4003D60: main (main.c:37)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants