per-module per-thread constructors and destructors

Frank Cusack fcusack at fcusack.com
Wed Aug 31 00:07:33 CEST 2005


On August 30, 2005 5:22:16 PM -0400 Alan DeKok <aland at ox.org> wrote:
> Frank Cusack <fcusack at 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



More information about the Freeradius-Devel mailing list