Обсуждение: Re: [pgsql-hackers-win32] Win32 open items

Magnus Hagander wrote:
> It won't work unless you make it a copy, I think. Because you'd have to
> lock the entire structure whenever you called PQexec() (since you don't
> know when PQexec does things), and that's just the time when you'd need
> to use it from the other thread...
>
> As for making a copy, yes, that's the other optino I had, but completely
> had lost when I wrote that mail. However, it cannot be done in just
> pqsl, because psql doesn't see the contents of PGconn. We'd need at
> least one new libpq function to create a duplicate PGconn. In order to
> be safe, this should probably be a for-cancel-only-copy, so we shouldn't
> copy the whole PGconn. But we could copy whatever fields are required to
> cancel the query, which in turn would require a new cancelilng function
> that accepted this limited structure.
>
> Probably doable, but it requires API additions, I think.

I thought about this.  There are a lot of pointers in PGconn, and most
we don't use so I don't like the idea of adding a new API to copy the
complex PGconn structure just for Win32 psql cancel.  Looking at the
PQrequestCancel() code, it only writes to errorMessage and reads from
everything else, and I don't see any of those changing (except
errorMessage) once the connection is established, so we could do a
non-deep copy of the structure and reuse all the existing structure
pointers.  We would just need to create a new errorMessage structure
because PQrequestCancel() wants to write to that.

We do need to use locking while we create/destroy this structure.

> > I can't think of how to simulate the longjump() currently
> > used when cancelConn is NULL though.
>
> Um. No need for that on win32, I think. When cancelConn is NULL (I am
> now assuming we have solved the fact that cancelConn has to be a copy),
> we can just do a "return" from the signal handler. The therad will then
> happily end, and we're all fine. Or is it supposed to do something other
> than just ignoring the cancel request if cancelconn is NULL?

Agreed, we can just ignore it.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Magnus Hagander wrote:
>> Probably doable, but it requires API additions, I think.

> I thought about this.  There are a lot of pointers in PGconn, and most
> we don't use so I don't like the idea of adding a new API to copy the
> complex PGconn structure just for Win32 psql cancel.  Looking at the
> PQrequestCancel() code, it only writes to errorMessage and reads from
> everything else, and I don't see any of those changing (except
> errorMessage) once the connection is established, so we could do a
> non-deep copy of the structure and reuse all the existing structure
> pointers.  We would just need to create a new errorMessage structure
> because PQrequestCancel() wants to write to that.

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 ;-))

            regards, tom lane