Opened 4 years ago

Closed 4 years ago

#16741 closed defect (fixed)

recv_ni() and friends don't ask about errno the Windows way?

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We have a report from a Windows relay operator about many failures in drain_fd():
https://lists.torproject.org/pipermail/tor-relays/2015-August/007557.html

A) Looking through src/common/compat_threads.c I see that sock_drain() (which is what drain_fd uses if we decided socketpair was the best way to make a pipe) calls recv_ni() which flat-out just looks at errno, with none of the usual Windows wrapping to get at the errno. Is this a bug?

B) Can we please have some more comments in compat_threads.c? What is sock_drain for? Why does replyqueue_process() appear to read and then discard everything the worker has to say?

Thanks!

Child Tickets

Change History (4)

comment:1 in reply to:  description Changed 4 years ago by someone_else

Replying to arma:

A) Looking through src/common/compat_threads.c I see that sock_drain() (which is what drain_fd uses if we decided socketpair was the best way to make a pipe) calls recv_ni() which flat-out just looks at errno

Windows socket functions don't set errno. So this is a bug.

comment:2 Changed 4 years ago by nickm

I don't see how this would explain the behavior observed on the original email, unless there are a whole load of WSAEINTR results, which shouldn't be possible. I suspect there's another error here too.

Still, the current code *is* wrong. Branch bug16741_026 in my public repository checks for EINTR properly, adds comments, and logs the actual error encountered.

comment:3 Changed 4 years ago by nickm

Status: newneeds_review

comment:4 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to master; if this helps us find a real bug, that bugfix will be backportable.

Note: See TracTickets for help on using tickets.