Thread: Re: Win32 open items

Re: Win32 open items

From
"Magnus Hagander"
Date:
>I think Magnus had the right idea.  We should invent a completely
>separate opaque struct that contains *only* the fields that
>PQrequestCancel actually needs (the hostname, port, and secret key;
>anything else?) and make a function to create one of these from a
>PQconn.  This struct could then be read-only as far as the thread-safe
>variant of PQrequestCancel is concerned.
>
>The error message return convention used by PQrequestCancel
>leaves a lot
>to be desired as well; I'd be inclined to think of something else for
>the new variant of PQrequestCancel.  Possibly have the caller supply a
>buffer to write into.
>
>We could probably reimplement the existing PQrequestCancel on
>top of the
>cleaner version, or at least find some way to share code.  But
>basically, the API it has now is pretty bogus.  (I think I can
>say that,
>since I invented it ;-))


Here is an attempt at this. First patch contains the changes to libpq,
second patch contains changes to psql to use this API. Docs not updated
yet, pending approval of the general idea at least :)

(Tested on win32 and slackware linux)

//Magnus

Attachment

Re: Win32 open items

From
Tom Lane
Date:
"Magnus Hagander" <mha@sollentuna.net> writes:
> Here is an attempt at this. First patch contains the changes to libpq,
> second patch contains changes to psql to use this API. Docs not updated
> yet, pending approval of the general idea at least :)

I think it would be better to dispense with the PQgetCancelError
function and just make the signature of PQcancel be
    int PQcancel(PGcancel *cancel, char *errbuf, int errbuflen);
where errbuf would normally point to a local array in the calling
function.

As-is, PQcancel is itself not thread safe because it is scribbling
on the PGcancel struct.  I thought the whole point of this exercise
was to allow multiple threads to use the PGcancel struct; which seems
to me to imply that it had better be read-only to PQcancel.

We don't need the cancelConnLock if this is done properly (at least,
assuming that storing a pointer is atomic, which seems reasonable).

            regards, tom lane