Thread: Thread Safety of ODBC Driver
I was surprised to find a message in the archive indicating that the PostgreSQL ODBC driver was not thread safe. After running through the source, I was able to confirm that the driver indeed isn't thread safe. Anyway, I'm curious if anyone is working on making the driver thread safe. I noticed that some work has been done on the driver provided with unixODBC (localtime and localtime_r ifdefs), but it still misses a few things I found on a quick read through the source. I have a threaded application running on FreeBSD with iODBC that I need to get working, and I'd rather not duplicate effort if someone already has patches for the driver. Thanks, Jason Koeninger J&J Computer Consulting http://www.jjcc.com
> -----Original Message----- > From: Jason Koeninger > > I was surprised to find a message in the archive indicating that the > PostgreSQL ODBC driver was not thread safe. After running through > the source, I was able to confirm that the driver indeed isn't thread > safe. > > Anyway, I'm curious if anyone is working on making the driver > thread safe. It's on my personal TODO but I haven't had much time to develop psqlodbc recently. > I noticed that some work has been done on the driver provided with > unixODBC (localtime and localtime_r ifdefs), but it still misses > a few things I > found on a quick read through the source. As for static variables, I have removed most of them in my local source. As for non thread-safe functions, please point out how to change them. Maybe I would be able to reflect the change to CVS. regards, Hiroshi Inoue
Here's a list of the issues I found. Since you've dealt with the statics, I won't worry about those. connection.c: Uses strtok. Could convert to strtok_r. On FreeBSD, I used strsep. convert.c: localtime used in copy_and_convert_field and copy_statement_with_parameters. Use localtime_r and create another struct tm to use as the extra argument. environ.c: The "conns" global at the start of the file could be a problem. Looks like the only reason it's used is to commit all connections on a single HENV at once if SQLTransact receives a NULL HDBC. If it's necessary that the array exist, you'll have to serialize access somehow. execute.c: Uses the conns global in PGAPI_Transact. gpps.c: The Unix implementation of Windows INI files has a couple functions I'm not as familiar with, but I think they may be a problem. getpwuid and getuid. I believe they return pointers to statics, don't they? misc.c: Also has some getpwuid and getuid calls. mylog and qlog probably need to be serialized, but they shouldn't cause a crash. socket.c: Uses gethostbyname. Some platforms have a gethostbyname_r, but the implementations vary. I know in my class libraries, I have a different implementation across Linux, Solaris, FreeBSD, Windows, and OS/2. On FreeBSD, you can use mutexes to serialize gethostbyname or use the function getipnodebyname. The others either have a gethostbyname that returns thread local storage or a gethostbyname_r variation. The problem with these fixes is that, at least in FreeBSD, you have to compile with the -pthread option to get some of the normal C library functions to work safely in a threaded environment. Some of the functions are also only defined in libc_r so some of these fixes may require precompiler directives and makefile changes if you want a thread-safe driver. Let me know if there's anything I can do to help. I'm also going to be looking into modifying the code to let me set the transaction isolation level to serializable. If you have any recommendations or preferences in that area, please let me know. Best Regards, Jason Koeninger J&J Computer Consulting http://www.jjcc.com On Sat, 7 Sep 2002 07:31:08 +0900, Hiroshi Inoue wrote: >> -----Original Message----- >> From: Jason Koeninger >> >> I was surprised to find a message in the archive indicating that the >> PostgreSQL ODBC driver was not thread safe. After running through >> the source, I was able to confirm that the driver indeed isn't thread >> safe. >> >> Anyway, I'm curious if anyone is working on making the driver >> thread safe. > >It's on my personal TODO but I haven't had much time to >develop psqlodbc recently. > >> I noticed that some work has been done on the driver provided with >> unixODBC (localtime and localtime_r ifdefs), but it still misses >> a few things I >> found on a quick read through the source. > >As for static variables, I have removed most of them in my local source. >As for non thread-safe functions, please point out how to change them. >Maybe I would be able to reflect the change to CVS. > >regards, >Hiroshi Inoue > >
Sorry for the delay. I have little time to spare for psqlodbc now. I've developped psqlodbc mainly from Windows side. I've planned a trial thread-safe implementation under Windows. As for unix I knew about a few xxx_r functions but I don't know details about it and unfortunately there's no developper from unix side. I'm very happy if someone would take care of the driver from unix side. Jason Koeninger wrote: > > Here's a list of the issues I found. Since you've dealt with the statics, > I won't worry about those. > > connection.c: Uses strtok. Could convert to strtok_r. On FreeBSD, > I used strsep. > > convert.c: localtime used in copy_and_convert_field and > copy_statement_with_parameters. Use localtime_r and create > another struct tm to use as the extra argument. > > environ.c: The "conns" global at the start of the file could be a > problem. Looks like the only reason it's used is to commit all > connections on a single HENV at once if SQLTransact receives > a NULL HDBC. If it's necessary that the array exist, you'll have > to serialize access somehow. > > execute.c: Uses the conns global in PGAPI_Transact. My plan under Windows is to use CriticalSection to serialize the access to conns global. > gpps.c: The Unix implementation of Windows INI files has a > couple functions I'm not as familiar with, but I think they may be > a problem. getpwuid and getuid. I believe they return pointers > to statics, don't they? gpps.c is used with neither iDOBC nor unixODBC. I have no interest for standalone driver. > misc.c: Also has some getpwuid and getuid calls. mylog and > qlog probably need to be serialized, but they shouldn't cause a > crash. I would also use another CriticalSection to serialize the access to mylog or qlog. > socket.c: Uses gethostbyname. Some platforms have a > gethostbyname_r, but the implementations vary. I know in my > class libraries, I have a different implementation across > Linux, Solaris, FreeBSD, Windows, and OS/2. On FreeBSD, > you can use mutexes to serialize gethostbyname or use the > function getipnodebyname. The others either have a gethostbyname > that returns thread local storage or a gethostbyname_r > variation. > > The problem with these fixes is that, at least in FreeBSD, you > have to compile with the -pthread option to get some of the > normal C library functions to work safely in a threaded environment. > Some of the functions are also only defined in libc_r so some of > these fixes may require precompiler directives and makefile changes > if you want a thread-safe driver. The development of psqlodbc was moved to GBorg (http://gborg.postgresql.org/project/psqlodbc/projdisplay.php) and psqlodbc could have its own release cycle. As for Windows there's no problem. However as for *nixes, the *make* may be broken. I'm happy if you can figure out how to manage it together with the thread-safe option. regards, Hiroshi Inoue http://w2422.nsk.ne.jp/~inoue/
On Mon, 09 Sep 2002 10:59:59 +0900, Hiroshi Inoue wrote: >I've developped psqlodbc mainly from Windows side. >I've planned a trial thread-safe implementation >under Windows. Ahh...thanks for the information. I was coming completely from the Unix side and wasn't thinking at all about Windows....as you noticed. >As for unix I knew about a few xxx_r functions but >I don't know details about it and unfortunately >there's no developper from unix side. Looks like it will probably be me then... ;-> >Jason Koeninger wrote: >> >> execute.c: Uses the conns global in PGAPI_Transact. > >My plan under Windows is to use CriticalSection to serialize >the access to conns global. That's probably the most straightforward approach on Windows, but I don't believe there's an equivalent on Unix. We'll probably have to use System V semaphores or Pthread mutexes. >gpps.c is used with neither iDOBC nor unixODBC. >I have no interest for standalone driver. Thanks for the heads up. I was in a hurry and just read every .c and .h file not paying attention to whether or not they were actually used. Needless to say, I found it odd that the driver would have its own ini file functions. >> misc.c: Also has some getpwuid and getuid calls. mylog and >> qlog probably need to be serialized, but they shouldn't cause a >> crash. > >I would also use another CriticalSection to serialize the >access to mylog or qlog. Critical sections are reliable ways to move from single to multi-threaded support for functions that have to be serialized, but given the number of times both functions are called in the code, it might be worthwhile to consider other options (log thread or semaphores). For client applications, I don't see it being a huge issue. >*nixes, the *make* may be broken. I'm happy if you >can figure out how to manage it together with >the thread-safe option. I'll look more closely at the compile issues as I have time and see what I can figure out. I read the GBorg page the other day, but I'll go back and take a look at the procedures for submitting patches. Best Regards, Jason Koeninger J&J Computer Consulting http://www.jjcc.com
> -----Original Message----- > From: Jason Koeninger [mailto:jkoenin@jjcc.com] > Sent: 09 September 2002 03:38 > To: Hiroshi Inoue > Cc: pgsql-odbc@postgresql.org > Subject: Re: [ODBC] Thread Safety of ODBC Driver > > >I read the GBorg page the other day, but > I'll go back > and take a look at the procedures for submitting patches. Hi Jason, The Gborg site is a a 'work in progress' as we move over there. I'm not sure how we should handle patches yet - Hiroshi, I can think of 3 options: 1) Use pgsql-patches@postgresql.org - I don't read that list at the moment so someone else would need to keep an eye on it. 2) Use pgsql-odbc@postgresql.org - We all listen in here. 3) I can switch on the patch manager in Gborg - If I remember correctly, that will require one of us to check for new patches reguarly. I can probably get that modified to notify the list upon upload though... Any thoughts? Regards, Dave.