Thread: Re: Win32 open items
>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
"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