[Proj] libproj4 thread safety

[Patrick Mézard]

On Wed, 23 Feb 2005 10:04:41 -0500, Gerald Evenden
<gerald.evenden at verizon.net> wrote:
> I am not familiar with the concept of threads so I cannot give a
> meaningful
> answer to the question but I will make the following comments.  

Threading is a complex thing but I will try to summarize the issues
with libproj4. Basically, in a multithreaded environment the library
interface may be used at the same time from several execution contexts
- threads. Assuming your library does nothing fancy with threading
primitives or thread-unsafe functions, the whole issue just summarize
to shared objects access. Lets take 2 threads T1 and T2 calling an
hypothetical libproj4 function pj_func() :

The code looks like:

--
pj_func();
if(pj_errno)
    printf("error");
--

An execution trace may looks like : 
T1: pj_func(); //Succeeds
T2: pj_func(); //Fails too, write pj_errno
T1: if(pj_errno); //Oops reading T2 pj_errno value
T1:    printf("error");

Here, T1 misdetected a function call failure because it read a shared
value overwritten by T2 between its own function call and the moment
it checks the variable (this is a race condition). There are 2
problems:
1- pj_errno is written from 2 threads simultaneously which usually
means undefined behaviour.
2- the function call and the pj_errno checking are not wrapped in a
single transaction.

(1) and (2) can be solved with return codes for instance because
return codes are allocated on the stack, and stacks are local to
threads so return values are not shared between threads. I know it
cannot be done right now.

So very simplistic advices to make the library thread-safe are:
- No mutable global variables
- Call thread-safe functions only (avoid C-runtime strtok on systems
it is unsafe for instance).
- If you have mutable global variables, it means they belong to thread
execution context. Wrap them in a structure every thread is supposed
to create before using the library, and modify the function prototypes
to take this structure is argument.

I will detail changes which can be made to libproj4 without changing
the library interface too much.

>The
> pj_errno
> is used during the initialization call because there is not any other
> way to return a reason for failure to return a viable projection
> pointer.
> Presumable, an error condition argument could be added to the
> initialization
> call but a great deal of checking of internal procedures would be
> required
> to ensure that this method will work.

Ok, pj_errno can be solved differently. I said it could be replaced by
return codes but it would break too many things in libproj4. Most
multithreading environment support the concept ot thread local storage
(TLS), which is some kind of memory location to store thread-global
variables. pj_errno can be easily replaced by such a thing, but light
changes must be made to the variable declaration: replace the pj_errno
variable with accessors and maybe add library
initialization/unitialization functions for some implementations
(unnecessary with Win32, might be helpful for pthreads).

> When an error occurs with an open projection the returned coordinates
> are
> flagged with HUGE values and, in some conditions, pj_errno is set.  I
> admit that
> this can be confusing when more that one projection pointer is active
> and it
> would seem that this condition could be alleviated with a 'errno' flag
> within
> the structure and limit pj_errno's use to initialization call.
> 
> This change looks reasonably easy by merely altering F_ERROR and I_ERROR
> macros and the basic definition of struct PJconsts.  There would have
> to be a
> review of code for other situations occuring during forward/inverse
> calls.
> 
> I patterned the opening procedures after C file procedures where a
> failure
> to open the file resulted in a null functional value and an error code
> in the
> external errno.  Admittedly, I did not follow trough with equivalents to
> feof, ferror and clearerr  Clearerr would not be necessary as any
> subsequent
> call to forward/inverse would clear the error condition.
> 
> Any change in this area would be significant release change and not
> just a
> trivial adding of projections of correcting old projections.
> 
> A consensus of users of libproj4 is needed on this issue.

I completely agree with you, and I try to find a way to minimize
interface changes.

The other issue was related to the structures returned by pj_init, the
PJs. Once they are allocated, are they modified in any way ? If not
they can be shared between threads easily, otherwise there must be one
instance for every projection per thread. I ask this because your
proposal to add a pj_errno member to PJs typically make them
thread-unsafe. I guess I would have to read the code to ascertain
that.

To summarize : 
1- For a win32 implementation, I need nothing to make pj_errno
thread-safe. I just have to add platform dependant declaration
specifiers to the declaration/definition.
2- For a generic implementation, I suppose that a library
initializer/uninitializer and a get_pj_proj()/set_pj_proj(int)
functions should be declared, and implemented for every target
platform.
3- If there are other threading issues, the usual solution is to have
an initialization function create a context structure before accessing
the library, then pass this context to every interface function. It
would require every prototype to be changed. It can be done without
changing existing functions, just tag them as thread-unsafe and make
them pass a static instance of the context to the thread-safe ones.

For the time being, I will stick with (1) and review the code or at
least parts I need (if I have enough free time :-)).

Patrick Mézard