multi: support verbose and err stream for conncache closure handle #3618
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcelRaad
approved these changes
Feb 26, 2019
|
The test 1506 output changes seem legit due to the new verbosity. |
|
I'm having second thoughts. Does anyone think it will be a problem to copy the stderr FILE stream from the easy handle? What if the handle is removed and the stream is closed or fd reused? That worries me. |
|
Yes I agree. The verbosity level I think is fairly safe but having the stderr pointer survive and "surprise" users after the original transfer is gone I think might lead to unhappiness. |
- Change closure handle to receive verbose setting rom the easy handle most recently added via curl_multi_add_handle. The closure handle is a special easy handle used for closing cached connections. It receives limited settings from the easy handle most recently added to the multi handle. Prior to this change that did not include verbose which was a problem because on connection shutdown verbose mode was not acknowledged. Ref: curl#3598 Closes #xxxx
jay
force-pushed
the
closure_handle_verbose
branch
from
6663aec
to
6337014
Compare
|
test still fails because the order isn't always the same -* Closing connection 3[CR][LF]
* Closing connection 1[CR][LF]
+* Closing connection 3[CR][LF]
* Closing connection 2[CR][LF]I'm trying strip |
|
You need to put that strip in the diff --git a/tests/data/test1506 b/tests/data/test1506
index 9e440907e..34a72cdf6 100644
--- a/tests/data/test1506
+++ b/tests/data/test1506
@@ -79,19 +79,22 @@ Host: %HOSTIP:%HTTPPORT
Accept: */*
</protocol>
<strip>
^Host:.*
-^\* Closing connection (?:1|2|3)$
</strip>
<file name="log/stderr1506" mode="text">
* Connection #0 to host server1.example.com left intact
* Connection #1 to host server2.example.com left intact
* Connection #2 to host server3.example.com left intact
* Closing connection 0
* Connection #3 to host server4.example.com left intact
+* Closing connection
+* Closing connection
+* Closing connection
</file>
<stripfile>
$_ = '' if (($_ !~ /left intact/) && ($_ !~ /Closing connection/))
+s/^(\* Closing connection) [123]/$1/
</stripfile>
</verify>
</testcase> |
jay
added a commit
to jay/curl
that referenced
this pull request
May 9, 2019
This reverts commit b0972bc. - No longer show verbose output for the conncache closure handle. The offending commit was added so that the conncache closure handle would inherit verbose mode from the user's easy handle. (Note there is no way for the user to set options for the closure handle which is why that was necessary.) Other debug settings such as the debug function were not also inherited since we determined that could lead to crashes if the user's per-handle private data was used on an unexpected handle. The reporter here says he has a debug function to capture the verbose output, and does not expect or want any output to stderr; however because the conncache closure handle does not inherit the debug function the verbose output for that handle does go to stderr. There are other plausible scenarios as well such as the user redirects stderr on their handle, which is also not inherited since it could lead to crashes when used on an unexpected handle. Short of allowing the user to set options for the conncache closure handle I don't think there's much we can safely do except no longer inherit the verbose setting. Bug: https://curl.haxx.se/mail/lib-2019-05/0021.html Reported-by: Kristoffer Gleditsch Ref: curl#3598 Ref: curl#3618 Closes #xxxx
jay
added a commit
that referenced
this pull request
May 10, 2019
This reverts commit b0972bc. - No longer show verbose output for the conncache closure handle. The offending commit was added so that the conncache closure handle would inherit verbose mode from the user's easy handle. (Note there is no way for the user to set options for the closure handle which is why that was necessary.) Other debug settings such as the debug function were not also inherited since we determined that could lead to crashes if the user's per-handle private data was used on an unexpected handle. The reporter here says he has a debug function to capture the verbose output, and does not expect or want any output to stderr; however because the conncache closure handle does not inherit the debug function the verbose output for that handle does go to stderr. There are other plausible scenarios as well such as the user redirects stderr on their handle, which is also not inherited since it could lead to crashes when used on an unexpected handle. Short of allowing the user to set options for the conncache closure handle I don't think there's much we can safely do except no longer inherit the verbose setting. Bug: https://curl.haxx.se/mail/lib-2019-05/0021.html Reported-by: Kristoffer Gleditsch Ref: #3598 Ref: #3618 Closes #3856
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
from the easy handle most recently added via curl_multi_add_handle.
The closure handle is a special easy handle used for closing cached
connections. It receives limited settings from the easy handle most
recently added to the multi handle. Prior to this change that did not
include verbose and the error stream which was a problem because on
connection shutdown verbose mode was not acknowledged.
Ref: #3598
Closes #xxxx