I started looking at how the protocol providers are hooked up to the core engine and came across this class that looks completely non- thread-safe to me.... am I missing something like upstream synchronization?

AFAICT so far there is one of these created for the ChangePasswordServer. It has an instance var env which has some initialization in the constructor.... but then all the functional methods do e.g:

public String addPrincipal( PrincipalStoreEntry entry ) throws Exception
    {
env.put( Context.PROVIDER_URL, catalog.getBaseDn ( entry.getRealmName() ) );

        try
        {
DirContext ctx = ( DirContext ) factory.getInitialContext ( env ); return ( String ) execute( ctx, new AddPrincipal ( entry ) );
        }
        catch ( NamingException ne )
        {
String message = "Failed to get initial context " + ( String ) env.get( Context.PROVIDER_URL );
            throw new ServiceConfigurationException( message, ne );
        }
    }

Unless something is going on to synchronize access to this class elsewhere 2 threads are going to be installing different PROVIDER_URLS at the same time...

I'd expect something like

public String addPrincipal( PrincipalStoreEntry entry ) throws Exception
    {
Hashtable<String, Object> env = new Hashtable<String, Object>(this.env);
env.put( Context.PROVIDER_URL, catalog.getBaseDn ( entry.getRealmName() ) );

        try
        {
DirContext ctx = ( DirContext ) factory.getInitialContext ( env ); return ( String ) execute( ctx, new AddPrincipal ( entry ) );
        }
        catch ( NamingException ne )
        {
String message = "Failed to get initial context " + ( String ) env.get( Context.PROVIDER_URL );
            throw new ServiceConfigurationException( message, ne );
        }
    }


is what is intended.

thanks
david jencks

Reply via email to