2008/4/17, Pierre Ynard wrote
:
> > On Wed, 16 Apr 2008, Yang Tse wrote:
> > > There has to be a _new_ user provided callback (abort check
> > > callback) which somehow will find out and tell to libcurl if libcurl
> > > should abort or not.
>
> Then it will look pretty much like the first patch that I posted. To
> which Daniel suggested I should in the contrary use the existing
> progress callback :)

Yes you are right, it would be much more like the first one. But there
are some subtle differences. Naming the callback type as
'curl_abortcheck_callback' and the var holding it as 'fabortcheck'
makes it more clear that the purpose of the callback is to check if
libcurl should abort or not. Not being relevant if it is called due to
an interrupt or to some other libcurl internal circumstance where it
would be appropriate to call it. While in your first patch you used
'CURLOPT_INTERRUPTFUNCTION' and 'finterrupt' which suggests it would
only be called as a reaction to an interrupt, and could also mislead
someone to think that this was actually a signal handler.

What's in a name.. ;-)

And relative to Daniel's previous suggestion. I understand that you
might not know which way to go now. I've also mentioned that I think
that Curl_pgrsUpdate() cannot be safely called in all cases that
Curl_socket_ready() and Curl_poll() are interrupted.

So maybe it would be better for your mental health to just follow
Daniel's suggestion of switching your app to the multi interface. :-)

> > 2008/4/16, Daniel Stenberg wrote:
> > If you really need this kind of check before every select()/poll()
> > call, isn't it just easier (and better) to just switch and use the
> > multi interface ?
>
> Well no, this check is not *needed*. Actually we may be thinking the
> wrong way: this extra check is just a hack to make up for the lack of
> proper, safe signal handling with signal masks and ppoll()/pselect().
> It would eventually not be there at all on platforms that support
> ppoll()/pselect(). So, as "just a hack," it should not decide how the
> main stuff is implemented.

He, he. I can see this from another point.

You are trying to provide some functionality that tries to
dramatically minimize the time required to react to an abort
condition. If _many_ systems are not going to be able to benefit from
it and it increases complexity then clearly it should not be adopted.

The check previous to select()/poll() is not 'just a hack' it exists
for several reasons.

On systems on which signals cannot be masked it is there to provide
the abort capability when the signal is triggered previous to the call
to select()/poll(), otherwise select()/poll() will consume the full
timeout and it would make no sense to provide a capability that only
works sometimes and increases code complexity. No need for patch.

And there is also the issue with systems which will consume the whole
timeout in select()/poll() even when a signal is triggered while
awaiting inside select()/poll() and this includes all _winsock_
systems and probably others. Eeeeeeek!

> So if I can't find a simpler, better way to work around the race
> conditions, I'm just going to remove this extra call altogether for
> now :) Independently of the availability of ppoll()/pselect(), proper
> signal blocking is enough to minimize race conditions, without the need
> for such an extra call.

Don't bother with it.

IMHO Follow Daniel's advice and switch to the multi interface, or live
with the at most 1 second delay.

-- 
-=[Yang]=-