Opened 4 years ago

Closed 3 years ago

#16355 closed enhancement (fixed)

[PATCH] Add usleep to data_impl functions

Reported by: cypherpunks Owned by: dgoulet
Priority: Very Low Milestone:
Component: Core Tor/Torsocks Version:
Severity: Normal Keywords: torsocks cpu spike hang
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The send_data_impl and recv_data_impl functions can enter an annoying
busy loop if a connection is laggy. Potentially if the connection
never establishes, this can continue for minutes, until the connection
times out, having at least one core running at 100% the entire time,
which is undesirable.

Adding a usleep(1000) is enough to make sure a torsocks'd program
doesn't 100% the CPU, and the user likely will not notice a 0.001
second delay.

Child Tickets

Attachments (2)

0001-Add-usleep-to-data_impl-functions.patch (1.5 KB) - added by cypherpunks 4 years ago.
patch: Add usleep to data_impl functions
sendrecv.patch (2.3 KB) - added by cypherpunks 4 years ago.
patch set to make send/recv_data_impl block rather than busy wait

Download all attachments as: .zip

Change History (6)

Changed 4 years ago by cypherpunks

patch: Add usleep to data_impl functions

comment:1 Changed 4 years ago by yawning

NACK. IMO the code should just use blocking I/O here, saving/restoring O_NONBLOCK before/after doing the socks handshake (using select or poll would also be less kludgy).

Changed 4 years ago by cypherpunks

Attachment: sendrecv.patch added

patch set to make send/recv_data_impl block rather than busy wait

comment:2 Changed 4 years ago by cypherpunks

Here's a patch set to make send/recv_data_impl block rather than busy wait in the EAGAIN/EWOULDBLOCK case. It also makes them returns EIO, rather than whatever error code 1 is, in the case of a short read before EOF.

If there is a concern the file descriptor numbers may be arbitrarily high here, it could be changed to use poll instead of select. I choose select by default in portable code because poll is known to be broken on certain platforms, particularly OS X -- although in this case, the file descriptor may always be a socket, in which case OS X poll is not broken.

comment:3 Changed 3 years ago by dgoulet

Status: newaccepted

Accept a bunch of tickets for torsocks.

comment:4 Changed 3 years ago by dgoulet

Resolution: fixed
Severity: Normal
Status: acceptedclosed

I've merged the sendrecv patch. I moved the whole select in a common function called wait_on_fd. The rest is as is. Thanks!

Note: See TracTickets for help on using tickets.