sustrik / libdill

Structured concurrency in C
MIT License
1.68k stars 155 forks source link

ENOTCONN isn't a fatal error #111

Closed vlm closed 7 years ago

sustrik commented 7 years ago

Right. It shouldn't crash. However, it should return the error to the user given that it means that orderly shutdown cannot be performed.

sustrik commented 7 years ago

I've committed a patch that should solve this problem. Please, check.

vlm commented 7 years ago

Doesn't it need to hclose() the socket if tcp_close() returns -1?

vlm commented 7 years ago

(Added a few characters to the patch to get higher quality error feedback during assertions).

sustrik commented 7 years ago

I am not sure whether the comment above referred to this patch, but if tcp_close() fails, it should close the socket. In other words, after tcp_close() returns, the socket is no longer valid, irrespective of success or failure of the function.

vlm commented 7 years ago

Then your patch is incorrect (or rather not enough). Because it returns -1 whereas the socket stays put, albeit in an invalid state. I think hclose() should be invoked there prior to returning -1.

vlm commented 7 years ago

The current patch we're commenting on is now just a bit of cosmetics around logging.

sustrik commented 7 years ago

Note that patch applies to hdone (which is supposed to close half a socket), not tcp_close().

As for the patch, I may be missing something but all I see is the old patch + a merge with current head.

vlm commented 7 years ago

Ok, sounds good. The patch is a single line change overall now. Let me rebase it quickly... Done. Check please.