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/

Search Discussions

  • Tom Lane at Nov 12, 2008 at 4:21 pm

    Andrew Chernow 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
  • Andrew Chernow at Nov 12, 2008 at 5:09 pm

    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 at Nov 13, 2008 at 5:51 pm

    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/
  • Tom Lane at Nov 13, 2008 at 5:45 pm

    Andrew Chernow 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
  • Andrew Chernow at Nov 13, 2008 at 6:58 pm

    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/
  • Andrew Chernow at Nov 14, 2008 at 12:15 pm

    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/
  • Tom Lane at Nov 14, 2008 at 11:00 pm

    Andrew Chernow 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>
  • Andrew Chernow at Nov 15, 2008 at 3:45 am

    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/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedNov 12, '08 at 2:40p
activeNov 15, '08 at 3:45a
posts9
users2
websitepostgresql.org...
irc#postgresql

2 users in discussion

Andrew Chernow: 6 posts Tom Lane: 3 posts

People

Translate

site design / logo © 2022 Grokbase