Socket not cleaned up on timeout; some socket descriptors cause crashes

[jrau14]

If I repeatedly try to open a connection with timeout specified, I eventually crash, with the message "Not enough bits to represent a signed value". The playground I used to isolate this issue looks like this:

import Socket

for i in 1...40 {
    let a = try Socket.create()
    print("iteration \(i)")
    do {
        try a.connect(to: "(REPLACE_ME)", port: 1234, timeout: 1000)
    } catch {
        print("connect error")
    }
    print("after connect")
}

Replace the IP with a valid host on the same network, but a port that isn't listening (I used 192.168.43.1 but that'll depend on your network I suppose).

I see the connect call block for the full 1000ms and then fail with an error each time, correctly so. After an arbitrary number of iterations (it doesn't seem consistent and I don't know why) it crashes with the error I mentioned above.

Here's the output from one run of that playground. I added a line to connect to print the value of socketDescriptor, so that's where the extra line per iteration comes from here:

iteration 1
Socket descriptor gotten is Optional(6)
connect error
after connect
iteration 2
Socket descriptor gotten is Optional(11)
connect error
after connect
iteration 3
Socket descriptor gotten is Optional(12)
connect error
after connect
iteration 4
Socket descriptor gotten is Optional(13)
connect error
after connect
iteration 5
Socket descriptor gotten is Optional(14)
connect error
after connect
iteration 6
Socket descriptor gotten is Optional(15)
connect error
after connect
iteration 7
Socket descriptor gotten is Optional(16)
connect error
after connect
iteration 8
Socket descriptor gotten is Optional(17)
connect error
after connect
iteration 9
Socket descriptor gotten is Optional(18)
connect error
after connect
iteration 10
Socket descriptor gotten is Optional(19)
connect error
after connect
iteration 11
Socket descriptor gotten is Optional(20)
connect error
after connect
iteration 12
Socket descriptor gotten is Optional(21)
connect error
after connect
iteration 13
Socket descriptor gotten is Optional(22)
connect error
after connect
iteration 14
Socket descriptor gotten is Optional(23)
connect error
after connect
iteration 15
Socket descriptor gotten is Optional(24)
connect error
after connect
iteration 16
Socket descriptor gotten is Optional(25)
connect error
after connect
iteration 17
Socket descriptor gotten is Optional(26)
connect error
after connect
iteration 18
Socket descriptor gotten is Optional(27)
connect error
after connect
iteration 19
Socket descriptor gotten is Optional(28)
connect error
after connect
iteration 20
Socket descriptor gotten is Optional(29)
connect error
after connect
iteration 21
Socket descriptor gotten is Optional(30)
connect error
after connect
iteration 22
Socket descriptor gotten is Optional(31)
Fatal error: Not enough bits to represent a signed value

If I look at the stack trace, I find myself at this snippet:
https://github.com/IBM-Swift/BlueSocket/blob/57d7e388dfc985a2fc301ed810752b707462294c/Sources/Socket/SocketUtils.swift#L154-L159

We are crashing on the line let mask = Int32(1 << bitOffset), where bitOffset is 31. I think that upper bit is a sign bit, so 1 << 31 is out of range for a signed Int32.

This reveals what I believe to be two issues:

1. Socket file descriptor values that cause bitOffset to be 31 will cause a crash--I think there are a few scenarios where this can happen, perhaps opening multiple connections in parallel?
2. My call to connect(to:port:timeout) failed with an Error, but the underlying socket file descriptor still exists--it seems connect isn't close()ing the socket it creates when it bails with an Error, in a few spots. I cannot close() this myself, as the value of socketDescriptor is only saved if the connection succeeds.

Please let me know if I've left out needed details. I would love to contribute a PR, but I need to get permission from our legal team before signing the CLA.

[billabt]

You’ve given me a GREAT explanation. I wish they were all so good. I’ve got some minor surgery tomorrow but hopefully I’ll be able to look at it by Friday unless you get your permission and submit a PR beforehand in which case I’ll try and merge it as soon as it passes CI. Thanks.

[AlanQuatermain]

Hm, sounds like I need to cast to UInt32 for the bitfield manipulation, then cast back to Int32 for the rest. Shouldn't be especially difficult.

[billabt]

Cool. Maybe add code to the test to test it out while you’re at it. 🤔

[AlanQuatermain]

All done. The pull request is up with the fix for the fd_set error, all CI tests ran successfully. This should also fix #123 and #125.

[AlanQuatermain]

…aaaand I've also added a fix for the orphaned socket descriptors now. That should about wrap everything up.

[billabt]

Great, I’ll try and do the merge tomorrow morning. Thanks for the quick response. How’s the leg? Feeling better I hope.

[billabt]

Fixed in 0.12.93.