Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

threaded-resolver: shutdown the resolver thread without error message #3630

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@bagder
Copy link
Member

bagder commented Feb 28, 2019

When a transfer is done, the resolver thread will be brought down. That
could accidentally generate an error message in the error buffer even
though this is not an error situationand the transfer would still return
OK. An application that still reads the error buffer could find a
"Could not resolve host: [host name]" message there and get confused.

Reported-by: Michael Schmid
Fixes #3629

threaded-resolver: shutdown the resolver thread without error message
When a transfer is done, the resolver thread will be brought down. That
could accidentally generate an error message in the error buffer even
though this is not an error situationand the transfer would still return
OK.  An application that still reads the error buffer could find a
"Could not resolve host: [host name]" message there and get confused.

Reported-by: Michael Schmid
Fixes #3629

@bagder bagder added the name lookup label Feb 28, 2019

@jay

This comment has been minimized.

Copy link
Member

jay commented Mar 1, 2019

I'm not opposed to this but also I don't like the precedent. It's clearly documented that the error buffer is not valid on success.

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Mar 1, 2019

I completely agree. The man page is crystal clear on this detail and I think any user who expects something different is wrong. However, we have started to slowly try to remedy this (returning data in the buffer when no error is returned) and this fix is one step along that path.

@bagder bagder closed this in 754ae10 Mar 1, 2019

@bagder bagder deleted the bagder/resolve-thread-shutdown branch Mar 1, 2019

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Mar 6, 2019

Related to this, @jay filed this PR on the PHP code itself to not show any error string unless an error was previously found: php/php-src#3901 - now merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.