libpq-events windows gotcha
Just noticed that the last libpqtypes release was broken on windows when
dynamically linking. The problem is that windows has two addresses for
functions, the import library uses a stub "ordinal" address while the
DLL itself is using the real address; yet another m$ annoyance. This
breaks the PQEventProc being used as a unique lookup value.
Be aware that there is nothing wrong with the libpq-events API. One
just has to be very careful on windows with DLLs.
libpqtypes fixed this issue by no longer publically exposing its
PQEventProc, didn't need to do this anyways. libpqtypes instanceData is
meant to be private. Version 1.2b conatins this fix and will be online
soon.
// this used to be a macro
int PQtypesRegister(PGconn *conn)
{
return PQregisterEventProc(conn, pqt_eventproc, "pqtypes", NULL);
}
// used to be a public function named PQtypesEventproc, now internal
int pqt_eventproc(PGEventId id, void *info, void *passThrough)
This is a big gotcha for any libpq-events implementors. It should
probably be documented in some fashion. Other implementations may want
to expose the PGEventProc so its API users can access the instanceData
... so they can get the lookup key. For this case, a public function
that returns the address of the private "static not dllexport"
PGEventProc would be required.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Andrew Chernow <ac@esilo.com> writes:
Just noticed that the last libpqtypes release was broken on windows when
dynamically linking. The problem is that windows has two addresses for
functions, the import library uses a stub "ordinal" address while the
DLL itself is using the real address; yet another m$ annoyance. This
breaks the PQEventProc being used as a unique lookup value.
This is a big gotcha for any libpq-events implementors. It should
probably be documented in some fashion.
Hmm. Well, it's not too late to reconsider the use of the function
address as a lookup key ... but what else would we use instead?
regards, tom lane
Tom Lane wrote:
Andrew Chernow <ac@esilo.com> writes:
Just noticed that the last libpqtypes release was broken on windows when
dynamically linking. The problem is that windows has two addresses for
functions, the import library uses a stub "ordinal" address while the
DLL itself is using the real address; yet another m$ annoyance. This
breaks the PQEventProc being used as a unique lookup value.
This is a big gotcha for any libpq-events implementors. It should
probably be documented in some fashion.Hmm. Well, it's not too late to reconsider the use of the function
address as a lookup key ... but what else would we use instead?regards, tom lane
Here are the options we see:
1. PQregisterEventProc returns a handle, synchronized counter
incremented by libpq. A small table could map handle value to proc
address, so register always returns the same handle for a provided
eventproc. Only register would take an eventproc, instanceData
functions would take this magical handle.
2. string key, has collision issues (already been ruled out)
3. have implementors return a static variable address (doesn't seem all
that different from returning a static funcaddr)
4. what we do now, but document loudly that PGEventProc must be static.
If it can't be referenced outside the DLL directly then the issue
can't arise. If you need the function address for calls to
PQinstanceData, an implementor can create a public function that returns
their private PGEventProc address.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Andrew Chernow wrote:
Tom Lane wrote:
Andrew Chernow <ac@esilo.com> writes:
Just noticed that the last libpqtypes release was broken on windows
when dynamically linking. The problem is that windows has two
addresses for functions, the import library uses a stub "ordinal"
address while the DLL itself is using the real address; yet another
m$ annoyance. This breaks the PQEventProc being used as a unique
lookup value.
This is a big gotcha for any libpq-events implementors. It should
probably be documented in some fashion.Hmm. Well, it's not too late to reconsider the use of the function
address as a lookup key ... but what else would we use instead?regards, tom lane
Here are the options we see:
1. PQregisterEventProc returns a handle, synchronized counter
incremented by libpq. A small table could map handle value to proc
address, so register always returns the same handle for a provided
eventproc. Only register would take an eventproc, instanceData
functions would take this magical handle.2. string key, has collision issues (already been ruled out)
3. have implementors return a static variable address (doesn't seem all
that different from returning a static funcaddr)4. what we do now, but document loudly that PGEventProc must be static.
If it can't be referenced outside the DLL directly then the issue can't
arise. If you need the function address for calls to PQinstanceData, an
implementor can create a public function that returns their private
PGEventProc address.
Do you have a preference form the list above, or an another idea? If
not, we would probably implement #1. Although, the simplest thing is #4
which leaves everything as is and updates the sgml docs with a strong
warning to implementors.
I am not sure which is best.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Andrew Chernow <ac@esilo.com> writes:
Here are the options we see:
1. PQregisterEventProc returns a handle, synchronized counter
incremented by libpq. A small table could map handle value to proc
address, so register always returns the same handle for a provided
eventproc. Only register would take an eventproc, instanceData
functions would take this magical handle.2. string key, has collision issues (already been ruled out)
3. have implementors return a static variable address (doesn't seem all
that different from returning a static funcaddr)4. what we do now, but document loudly that PGEventProc must be static.
If it can't be referenced outside the DLL directly then the issue can't
arise. If you need the function address for calls to PQinstanceData, an
implementor can create a public function that returns their private
PGEventProc address.
Do you have a preference form the list above, or an another idea? If
not, we would probably implement #1. Although, the simplest thing is #4
which leaves everything as is and updates the sgml docs with a strong
warning to implementors.
I think #1 is mostly going to complicate life for both libpq and callers
--- libpq has to deal with generating the handles (threading issues
here) and callers have to remember them somewhere. And it's not even
clear to me that it fixes the problem: wouldn't you get two different
handles if you supplied the internal and external addresses of an
eventproc?On the whole I vote for #4 out of these. I was just wondering whether
there were any better alternatives, and it seems there's not.
regards, tom lane
Tom Lane wrote:
And it's not even
clear to me that it fixes the problem: wouldn't you get two different
handles if you supplied the internal and external addresses of an
eventproc?
Both #1 and #4 suffer from this issue, internal & external register
methods. They also require the same WARNING in the docs. But, #1
solves the instancedata lookup issue. I am not trying to make a case
for #1 but it does appear to have a narrower failure window.
libpq has to deal with generating the handles
Well lock; if(not_in_map) handle = ++counter; unlock surely won't be to
difficult ;-)
(threading issues here)
There is no unregister, so the idea won't lock/unlock in high traffic
routines.
On the whole I vote for #4 out of these.
Okay.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
4. what we do now, but document loudly that PGEventProc must be static.
If it can't be referenced outside the DLL directly then the issue can't
arise. If you need the function address for calls to PQinstanceData, an
implementor can create a public function that returns their private
PGEventProc address.Do you have a preference form the list above, or an another idea? If
not, we would probably implement #1. Although, the simplest thing is #4
which leaves everything as is and updates the sgml docs with a strong
warning to implementors.On the whole I vote for #4 out of these.
I attached a patch for the docs. Its documented as a NOTE to the
PGEventProc.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Attachments:
libpq-events.patchtext/plain; name=libpq-events.patchDownload
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.269
diff -C6 -r1.269 libpq.sgml
*** doc/src/sgml/libpq.sgml 13 Nov 2008 09:45:24 -0000 1.269
--- doc/src/sgml/libpq.sgml 14 Nov 2008 12:08:07 -0000
***************
*** 5252,5263 ****
--- 5252,5275 ----
<para>
A particular event procedure can be registered only once in any
<structname>PGconn</>. This is because the address of the procedure
is used as a lookup key to identify the associated instance data.
</para>
+
+ <note>
+ <para>
+ On windows, functions can have two different addresses: one accessed
+ from outside a DLL, obtained from the import library, and another from
+ inside a DLL. For this reason, implementors should never directly expose
+ an event procedure. If the event procedure is needed by an API user,
+ then its address should be returned by a library function; ie.
+ <literal>PGEventProc MyGetEventProc(void);</literal>. This ensures that
+ the application and DLL are always using the same address.
+ </para>
+ </note>
</listitem>
</varlistentry>
</variablelist>
</sect2>
<sect2 id="libpq-events-funcs">
Andrew Chernow <ac@esilo.com> writes:
On the whole I vote for #4 out of these.
I attached a patch for the docs. Its documented as a NOTE to the
PGEventProc.
Applied, but I editorialized on the wording a bit. Let me know if you
think this is wrong ...
regards, tom lane
Index: libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.269
diff -c -r1.269 libpq.sgml
*** libpq.sgml 13 Nov 2008 09:45:24 -0000 1.269
--- libpq.sgml 14 Nov 2008 22:57:05 -0000
***************
*** 5255,5260 ****
--- 5255,5273 ----
<structname>PGconn</>. This is because the address of the procedure
is used as a lookup key to identify the associated instance data.
</para>
+
+ <caution>
+ <para>
+ On Windows, functions can have two different addresses: one visible
+ from outside a DLL and another visible from inside the DLL. One
+ should be careful that only one of these addresses is used with
+ <application>libpq</>'s event-procedure functions, else confusion will
+ result. The simplest rule for writing code that will work is to
+ ensure that event procedures are declared <literal>static</>. If the
+ procedure's address must be available outside its own source file,
+ expose a separate function to return the address.
+ </para>
+ </caution>
</listitem>
</varlistentry>
</variablelist>Tom Lane wrote:
Andrew Chernow <ac@esilo.com> writes:
On the whole I vote for #4 out of these.
I attached a patch for the docs. Its documented as a NOTE to the
PGEventProc.Applied, but I editorialized on the wording a bit. Let me know if you
think this is wrong ...
It's correct, the wording is clearer now. I wasn't aware there was a caution
tag, which is better than using <note>.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/