Threading libsmbclient - a proposal.

[Derrell Lipman]

On Thu, Apr 2, 2009 at 7:39 PM, Jeremy Allison <jra at samba.org> wrote:

> Andreas & Derrell,
>
> Just wanted to let you know I have a working design (and some
> preliminary code) for starting the process of properly
> threading libsmbclient.
>

Hi Jeremy,

Thanks for pursuing this! I'm a bit uncertain, though, where you're going
with it. If I understand you correctly, you're going to have libsmbclient
create these locks that apply deep down in the bowels of the core code. If
that's the case, it will mean that different threads could be blocked for a
long period of time awaiting receipt of the lock.

There should be no thread safety issues remaining in libsmbclient itself (as
long as one uses the lower-level interface where a context is passed to each
function, and not the POSIX compatibility interface that uses a static
context). Each thread allocates its own context, and libsmbclient should be
thread-sfae. The state that needs locking is much deeper in the code, and
locking it from within libsmbclient doesn't seem the correct place to do it.

Please elucidate a bit about how you anticipate this being used where you
expect the lock calls to be made, and what, specifically, needs locking
(i.e. where is the global state data currently). Based in ny current
understanding of where you're going with this, I believe I have a different
design that may be more efficient, but I'll first await your clarifications.

Thanks.

Derrell


> It's based on the ideas in openssl, but a little simpler.
>


> The idea is a caller desiring threads will have to initialize
> the thread state and locks we need by firstly calling :
>
> unsigned int SMBC_thread_num_locks()
>
> which will return the number of "big gloabl locks" (see
> below) we currently need and creating and initializing an
> array of this number of pthread_mutex_t's.
>
> Then they must call a set of functions :
>
> SMBC_thread_set_log_function()
> SMBC_thread_set_lock_function()
> SMBC_thread_set_dynamic_lock_function()
> SMBC_thread_set_dynamic_lock_create_function()
> SMBC_thread_set_dynamic_lock_destroy_function()
>
> which will be passed pointers to the callback functions
> libsmbclient will use internally to do the thread locking calls.
>
> Inside libsmbclient we'll start by adding "big
> global locks" (which we'll use to determine the
> number returned from SMBC_thread_num_locks()) and
> lock/unlock them internally by adding code
> like this to the internals of libsmbclient :
>
>        SMBC_TLOCK(GLOBAL_LOCKID);
>        ... access resource ....
>        SMBC_TUNLOCK(GLOBAL_LOCKID);
>
> Where SMBC_TLOCK will be a macro that expands to:
>        smbc_lock_(SMB_TUNLOCK,
>                        GLOBAL_LOCKID,
>                        __FILE__,
>                        __LINE__);
>
> and SMBC_TUNLOCK will expand to:
>        smbc_lock(SMB_TLOCK|SMB_TLOCK_WRITE,
>                        GLOBAL_LOCKID,
>                        __FILE__,
>                        __LINE__);
>
> GLOBAL_LOCKID will be a #define to the specific
> "big global lock" we're currently using. For example :
>
> #define GLOBAL_LOCKID_LIBRARY_INITIALIZED 1
> #define GLOBAL_LOCKID_NAMELOOKUP_CODE 2
>
> etc. To start with we could just use one :-).
>
> For areas where we need dynamic locks, we
> create them inside libsmbclient by :
>
>        void *plock = smbc_create_lock(const char *lockname, const char
> *file, int line);
>
> lock using the macro's :
>
>        SMBC_TDYNLOCK(plock);
>        ... access resource ...
>        SMBC_TDYNUNLOCK(plock);
>
> then delete using :
>
>        smbc_destroy_lock(void *plock, const char *file, int line)
>
> Note all these calls will be internal to libsmbclient
> and attendent libraries. I'm using TLOCK and TDYNLOCK
> prefixes so people don't get them confused with actual
> SMB lock calls.
>
> The really nice part of this design (from openssl)
> is that if you never set the callback functions this
> has *NO EFFECT* on existing code, and because we're
> hiding the pthread_mutex_t types behind an int index
> (for the global locks case) and a void pointer (in
> the dynamic case) there are *no dependencies* on
> any underlying threads library or primitives.
>
> For people who want pthreads we supply a module
> that provides a pthread sample implementation of
> all the callback functions above, but it's only
> linked in by people who want threading.
>
> I haven't started on the thread specific data part
> yet (which we'll need for the talloc_frame() calls),
> so I'll email more when I've thought about that some
> more.
>
> Comments / Questions ?
>
> Big thanks to Volker who got me looking at the
> openssl code in the first place !
>
> Jeremy.
>