On August 30, 2005 5:22:16 PM -0400 Alan DeKok <aland@ox.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
I need to add a thread local variable to rlm_otp.
I'm very, very, very, leery of adding thread-specific code to modules. It usually makes things much more complicated.
In this case, it makes it simpler. Handling a connection pool is much more complicated than simply adding ctor/dtor's.
The other program may return bar first, and even if it returned results in the order received, because both threads are blocked in read(), either might pick up the result for the other. An fd per thread fixes this. It also fixes the problem that writes() might interleave (atomic writes are only guaranteed in certain cases which won't be met here), but that is just icing for the cake.
You could try using a connection pool, like LDAP & SQL. On initialization, open one connection. When the module is called, it looks for a free connection, and if none exists, creates one.
OK, I looked at the LDAP module and it's unappealing. It's just overly complex for something that can be simple. (Similar to how losing init and destroy makes instantiate and detach more complex; although in that case it's not a big deal.) In fact, rlm_ldap gets it wrong. In ldap_get_conn(), conns[i].locked must not be tested until after a lock is obtained. rlm_sql seems to have a slight error as well, but it shouldn't manifest as a bug to the user. In sql_get_socket(), inst->last_used is not protected by a mutex. Oh, I see this is documented as acceptable. Both rlm_ldap and rlm_sql may have errors in connection teardown similar to the ones they have in setup. I didn't look. Note that connection pooling requires thread-specific code in the module as well. I'm going to go ahead and add ctor/dtor support. We can of course remove it if it looks too hairy. -frank