when WvHttpStream connects too fast
dcoombs
I've discovered that WvHttpStream doesn't work if it's able to connect
too quickly. This most often shows up when connecting to an http
server on the same physical machine. In this case, the connection can
be established *before* the content_source / input_stream is done
being read (1 K per callback) into a holding buffer in ::execute().
This results in the request never actually being sent to the server.
In contrast, ordinarily, the input stream is read, and then the
connection is established (resulting in another callback), resulting
in the request being sent.
I'm not certain what's the best way to completely fix this, but the
below workaround at least makes it 1024 times less likely to cause a
problem when the connection is established too quickly. In the spirit
of NinetyNinePercent, I submit this for your consideration. Anybody
have any better ideas?
Cheers,
-Dave
commit c9ba88f6f93c752dcba3d53c1b0a93c5575bbe82
Author: Dave Coombs <dco...@carillon.ca>
Date: Fri Jan 6 14:57:02 2012 -0500
Improving (but not quite fixing) wvhttpstream bug
If a wvhttpstream is too fast to connect (such as to a server
running on
the same machine), the connection may be established before
putstream is
done being read, in which case for some reason we don't get
another
callback, and the request doesn't get sent. We can at least
reduce the
chances of this happening.
diff --git a/urlget/wvhttpstream.cc b/urlget/wvhttpstream.cc
index 4de0cac..5db8528 100644
--- a/urlget/wvhttpstream.cc
+++ b/urlget/wvhttpstream.cc
@@ -399,6 +399,15 @@ void WvHttpStream::execute()
if(url->putstream->isok())
len = url->putstream->read(putstream_data, 1024);
+ // FIXME: If our stream is too fast to connect, we
won't
+ // get another execute to finish reading the
putstream, and
+ // we'll never send the request. Let's at least
reduce the
+ // chances of this causing a problem... although I'd
prefer
+ // to eliminate it completely, I'm not quite sure
how.
+ // -dcoombs (20120106)
+ if (len < 1024)
+ len = url->putstream->read(putstream_data, 1024);
+
if(!url->putstream->isok() || len == 0)
{
url->putstream = NULL;
Avery Pennarun
> --- a/urlget/wvhttpstream.cc
> +++ b/urlget/wvhttpstream.cc
> @@ -399,6 +399,15 @@ void WvHttpStream::execute()
> if(url->putstream->isok())
> len = url->putstream->read(putstream_data, 1024);
>
> + // FIXME: If our stream is too fast to connect, we
> won't
> + // get another execute to finish reading the
> putstream, and
> + // we'll never send the request. Let's at least
> reduce the
> + // chances of this causing a problem... although I'd
> prefer
> + // to eliminate it completely, I'm not quite sure
> how.
> + // -dcoombs (20120106)
> + if (len < 1024)
> + len = url->putstream->read(putstream_data, 1024);
> +
> if(!url->putstream->isok() || len == 0)
> {
> url->putstream = NULL;
What version of wvhttpstream are you looking at? In my current
master, the above code doesn't really exist (even the whitespace is
different). In fact, I think commit
0c3ca0ca2693e709d6f9c8711cb29bd15f95f15b might already solve your
problem.
Hope this helps,
Avery
Avery Pennarun
>> What version of wvhttpstream are you looking at? In my current
>> master, the above code doesn't really exist (even the whitespace is
>> different). In fact, I think commit
>> 0c3ca0ca2693e709d6f9c8711cb29bd15f95f15b might already solve your
>> problem.
>
> reason we *weren't* reading the putstream in its entirety on the first go.
> If there isn't, then so much the better.
Well, the "reason" is that it might be gigantic and it might
theoretically be coming from some network stream, so reading it all at
once would be blocking. But it didn't work at all if it didn't read
the whole thing at once (as you observed) and I don't know of any
existing apps that need to stream from one http stream to another, for
example, and we had to read it all into RAM anyway to get the
Content-Length, so eventually we just said screw it and tried to
eliminate the actually pressing class of bugs.
(This was part of the big batch of fixes for EQL Data, which uploads a
lot of stuff and uses WvHttpStream extensively. The latest version
should be a lot more stable in weird edge cases.)
Have fun,
Avery
Dave Coombs
> master, the above code doesn't really exist (even the whitespace is
> different). In fact, I think commit
> 0c3ca0ca2693e709d6f9c8711cb29bd15f95f15b might already solve your
> problem.
Yes, I think that commit will indeed fix my problem. I assumed there was a
reason we *weren't* reading the putstream in its entirety on the first go.
If there isn't, then so much the better.
Have fun,
-Dave