lldb-platform's listener socket only had a backlog of one connection.
That means that if more than one client connected simultaneously, the
connection would be refused. The test suite can be run remotely with
dozens of threads connecting simultaneously. Raised this limit to 100
to effectively eliminate lost connections.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Looks good but please consider my inline comment
include/lldb/Host/Socket.h | ||
---|---|---|
64 ↗ | (On Diff #22867) | I think you should leave the default value to be 1 unless you are sure nobody is depending on the original behavior. |
include/lldb/Host/Socket.h | ||
---|---|---|
64 ↗ | (On Diff #22867) | I agree, especially since the way this is used in ConnectionFileDescriptorPosix only accepts 1 connection. |
the way this is used in ConnectionFileDescriptorPosix only accepts 1 connection.
I think we should keep it at 5. ConnectionFileDescriptorPosix will reject the connection if it is coming from the wrong port. That means a port scanner or some other random solar flare could theoretically block the good connection from getting in. I know I'm way out on a limb here but consider this: what is the downside? The system resources are irrelevant and if the listener is closed (like in ConnectionFileDescriptorPosix), the backlog is flushed.
That said, I really don't care.
If you are confident that it won't cause any regression then I am fine with increasing it to 5 but I have no idea how many different part of the code base use this class.
Fair enough. Since not every incoming connection will be accepted it makes sense to try to ensure we get the right connection if possible.