On 11/18/10 11:37 PM, Vladislav Vaintroub wrote:
>
>
>> -----Original Message----- From: Davi Arnaut
>> [mailto:davi.arnaut@stripped] Sent: Thursday, November 18, 2010
>> 5:44 PM To: Vladislav Vaintroub Cc: commits@stripped
>> Subject: Re: bzr commit into mysql-5.5-runtime branch (davi:3186)
>> Bug#54790
>
>
>> I'll add a explanation. Essentially, it allows for a single, common
>> and shared implementation. If Windows XP supported WSAPoll(), I
>> would have gone for poll even there.
>>
> Ok.. Looking forward to fixing "kill" command bugs, I hope we'll
> not repeat libevent mistakes and introduce monsters like socketpairs
> to signal events (just because select() or WSAPoll() would not work
That's roughly the plan, at least for systems where socketpairs and
alike (eventfd actually) aren't monsters. I'm afraid of monsters :-O
> with pipes) . Old-fashioned WaitForSingle/MultipleObjects or a
> little more fancy asynchronous IO could be a better tool for the
> job :) But I digress, this has nothing to do with this patch.
Have you seen what a monster is I/O cancellation on Windows? =:O
I guess it's a good time to walk over what I've been thinking lately.
One thing to keep in mind is that buffers given to vio_read/vio_write
are owned by their callers. When those two functions are left, there
can't be any dangling reference to those buffers.
In the case of asynchronous I/O and given that we need to support
Windows XP, it seems the only option to support timeout and notification
(cancellation) is to actually close the socket on either cancellation or
timeout. The problem is that this creates a fancy discrepancy that I'm
not really sure is worth the trouble.
The alternative which I think is more sane and that shouldn't suck too
much performance wise is to use WSAEventSelect. But there is a catch.
The authors of the said function decided to make FD_READ events special
in the sense that they are level triggered, while FD_WRITE events are
edge triggered. Which leaves us with the choice to either disable
FD_READ events after each wait or to just let them pending and use
WSAEnumNetworkEvents after each wake up.
>
>>>> +int vio_timeout(Vio *vio, uint which, int timeout_sec)
>>> [skip]
>>>> + /* Deduce the current timeout status mode. */ + old_mode=
>>>> vio->write_timeout< 0&& vio->read_timeout< 0;
>>>
>>> Maybe more obvious variable name than old_mode (was_blocking?),
>>> or more explanation what and how was deduced.
>>
>> It is deducing a change in the mode of the timeout values, whether
>> both of them represent infinite timeouts. Blocking mode is on a
>> layer below, so it doesn't make much sense. I'll improve the
>> comment.
>
> OK, so the "mode" either some-timeout (== 0, either send or recv or
> both are timed operations), or no-timeout (==1, neither send nor
> receive are timed).
>
Yep.
[..]
>>>> + /* The first argument is ignored on Windows. */ + ret=
>>>> select(IF_WIN(0, fd + 1),&readfds,&writefds,&exceptfds,
>>>> timeout);
>>>
>>> It is already under #ifdef _WIN32, no IF_WIN required.
>>>
>>>> + IF_WIN(int, socklen_t) optlen= sizeof(error); +
>>>> IF_WIN(char, void) *optval= (IF_WIN(char, void) *)&error;
>>>
>>> Maybe there is a way to reduce 3 IF_WIN s to a single #ifdef
>>> _WIN32 ?
>>
>> Personally, the result looked more ugly two me. I would actually
>> prefer proper typedefs here. Do we set socklen_t to int on Windows?
>> I could introduce another type for the argument to setsockopt.
>
> SOCKET_SIZE_TYPE maybe, does it look close enough?
>
Seems it could work.
Regards,
Davi
