Avoid fcntl(O_NONBLOCK) calls on Linux

We already use the SOCK_CLOEXEC extesion to socket/accept4/open/socketpair on Linux so that we can create a socket/fd and make it close-on-exec in a single syscall. In 0.2.4.x, we might want to do the same for opening a nonblocking socket.

changed milestone to %Tor: 0.2.5.x-final

added component::core tor/tor easy milestone::Tor: 0.2.5.x-final priority::low resolution::implemented status::closed tor-relay type::enhancement labels

Trac:
Keywords: N/A deleted, easy added

Trac:
Keywords: easy deleted, easy tor-relay added

Trac:
Component: Tor Relay to Tor

Trac:
Milestone: Tor: 0.2.4.x-final to Tor: 0.2.5.x-final

Trac:
Type: defect to enhancement

Trac:
Username: piet

ticket5129.patch

Trac:
Username: piet

Wrote nonblocking versions of tor_open_socket() and tor_accept_socket() and used them throughout the code.

Trac:
Username: piet
Status: new to needs_review

This looks good. It should check the return values from set_socket_nonblocking() and from fcntl.

Unfortunately, it no longer applies cleanly to master, and will therefore need some hand-cleanup before I can merge it.

Trac:
Status: needs_review to needs_revision

Trac:
Username: piet

ticket5129.patch.v2

Trac:
Username: piet

This should do the trick, hopefully.

Trac:
Username: piet
Status: needs_revision to needs_review

Okay, looks better. Three things I want to do before merging this:

• The eventdns changes should be wrapped in an #if 1 or something -- eventdns.c is imported code (snarfed from Libevent 1.x and then patched sporadically with fixes from later Libevent versions), and we should minimize our changes to it. In fact, unless the old code is wrong , we should really just leave it alone.
• In several places, the patch uses an #ifdef _WIN32 to decide whether to call close() or closesocket(). Ideally, we should have a function or macro for that. This would be tor_close_socket, but without the socket accounting.
• This needs a changes/ file.

I'll see if I can do these in the next day or two, unless you want to.

Trac:
Status: needs_review to needs_revision

I can make the changes if you want. Not sure when I'll have the time, but it will be some time this week. I'm not sure whether I understand your first point correctly, though. You want to wrap the changed code in the #if branch, and leave the old code in the #else branch, so it won't be compiled but is still around for future patches? Is that correct?

Trac:
Username: piet

I think that sounds good, yes.

Trac:
Username: piet

ticket5129.patch.v3

Trac:
Username: piet

Trac:
Username: piet
Status: needs_revision to needs_review

Thanks, and sorry about the delay! (I hope the next version of trac sends notification mails when somebody uploads an attachment.)

I merged your patch as ebd4ab1506517b32ee3bd5bd1597b4373aa56ee7 ; fixed a bug as e25eb35f11b0419fe50791b3760f1d4016bf8a8b; and added tests in b8d9c8403718b2dca577947d2f5d9248ab87961e, d6adf055828283dda2ca9c0c77a80348778c45df, and bcc39c466688e41ac8f58e5589e854a916769a14. Thanks!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

closed

moved to tpo/core/tor#5129 (closed)