Re: dblib and thread safety

On Wed, Sep 22, 2004 at 09:23:37AM +0200, ZIGLIO, Frediano, VF-IT wrote:

> I see. However tds_ctx it's referenced in TDSSOCKET and used (only for
> reading) in all libTDS functions. So you have active readed structures

Whether or not it's used for reading or writing is not relevant. The
concern here is memory visibility, not mutual exclusion.

> while you have connections. If you call dbexit dblib free all
> connections so you "release" all pointers and you can safely delete
> context. In dbinit you allocate context so only dbinit can write to this
> context, it's not useful to initialize context inside the mutex. Perhaps
> we should check for no connections inside dbinit. It's an error to call

Actually, I thought about this too. My solution would be to use pthread_once() 
to ensure that dbinit() is only ever called once. 

This could also be achieved with a global flag (inside g_dblib_ctx) and a 
mutex if you wanted to make that same functionality available without threads 
enabled (probably a good idea).

> dbinit after another dbinit. It's also a programmer error to call dbinit
> while another dblib function it's called... I used no recursive mutex

Indeed, but I think we should guard for it anyway.

> cause are faster and cause my code don't need recursive one (if we
> require recursive one I have to change configure too). tds_convert use

Please don't get me wrong, I don't like using the recursive mutex for this 
patch as much as you do but without a major re-write there is no other way 
I can see to guarantee thread safety in that section of code.

In the context of something that communicates over TCP/IP sockets, I
also don't think the extra nano-second it takes to process a recursive 
mutex is worth discussing.

> every processor I know can read memory in a single instruction. It's not
> useful to use mutex here. Better to use volatile to disallow strange
> optmizations.

Wrong, wrong, wrong.

What if the other CPU (in a multi-processor computer) changes the value at 
the exact same instant in time as the read with the same atomic instruction?

You are also ignoring the effect of the TLB and cache.

Please see:

        
http://groups.google.com.au/groups?hl=en&lr=&ie=UTF-8&safe=off&selm=3895B019.7453%40LambdaCS.com

        
http://groups.google.com.au/groups?q=volatile+%2Bgroup:comp.programming.threads&hl=en&lr=&ie=UTF-8&safe=off&selm=QJZo9.17%24X93.717939%40news.cpqcorp.net&rnum=5

If you want to argue with me that's fine, but don't argue with Bil Lewis 
and David Butenhof, both of whom are the authors of the most respected
texts on threads.

I've programmed with threads for close to 5 years and have published
several papers on subject. I am very keen to get FreeTDS working reliably 
for my project and I don't want to have a long discussion on threads 
programming. 

Please, cut me some slack and let's get down to the business of making
dblib thread safe! :)

> For full thread safe for context you have to prevent a thread inside a
> dblib call and call to dbexit... however I don't know a solution for
> this... use a rw lock in every dblib ?

I still don't understand why we can not put a per-connection tds_ctx inside 
every individual DBPROCESS structure? I am no where near as familiar with the
internals of TDS as you are so is there some reason why you do not want to
do this?

Thanks for your help on this to date.

Cheers.