Announcement Announcement Module
Collapse
No announcement yet.
SecurityContextHolder propagation problem with thread pools Page Title Module
Move Remove Collapse
This topic is closed
X
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • SecurityContextHolder propagation problem with thread pools

    I am having a problem with the propagation of the SecurityContext into thread pools provided by the java.util.concurrency.Executors framework. I am using Java 1.6 and have tested this against both spring 2.5.6/spring security 2.0.5 and spring 3.0.0/spring security 3.0.0.

    It would seem that when I give a task to a thread pool the security context is passed correctly the first time the thread is used. But all subsequent calls that are given to that thread keep the original security context instead of propagating the new context.

    There is a workaround for this, which is to explicitly clear the context before exiting the thread. But this is a bit of a nasty gotcha if you don't know about it.

    My test program is made up of three classes.

    Code:
    public class Main {
        public static void main(String[] args) {
            SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL);
    
            ExecutorService threadPool = Executors.newFixedThreadPool(2);
    
            String[] nameList = { "Jack", "Jill", "James", "Steve", "Fred", "Olivia" };
    
            List<Thread> threadList = new ArrayList<Thread>();
    
            for (String name : nameList) {
                Thread thread = new Thread(new ParentThread(name, threadPool));
                thread.start();
                threadList.add(thread);
            }
    
            for (Thread thread : threadList) {
                try {
                    thread.join();
                } catch (InterruptedException e) {
                }
            }
    
            threadPool.shutdown();
        }
    }
    Code:
    public class ParentThread implements Runnable {
    
        private final ExecutorService mThreadPool;
    
        private final String mUser;
    
        public ParentThread(String user, ExecutorService threadPool) {
            mUser = user;
            mThreadPool = threadPool;
        }
    
        @Override
        public void run() {
            SecurityContext ctx = new SecurityContextImpl();
            ctx.setAuthentication(new UsernamePasswordAuthenticationToken(mUser, null));
            SecurityContextHolder.setContext(ctx);
    
            System.out.println("Parent: " + Thread.currentThread().getName() + " "
                    + SecurityContextHolder.getContext().getAuthentication().getPrincipal());
    
            mThreadPool.submit(new ChildThread(mUser));
        }
    }
    Code:
    public class ChildThread implements Runnable {
    
        private final String mUser;
    
        public ChildThread(String user) {
            mUser = user;
        }
    
        @Override
        public void run() {
            Object user = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
            System.out.println("Child: " + Thread.currentThread().getName() + " " + mUser + " = " + user);
        }
    }
    This gives output similar to the following where the first name for the child is the expected name and the second name is the actual name:
    Code:
    Parent: Thread-3 Steve
    Parent: Thread-0 Jack
    Parent: Thread-5 Olivia
    Child: pool-1-thread-2 Jack = Jack
    Child: pool-1-thread-2 Olivia = Jack
    Child: pool-1-thread-1 Steve = Steve
    Parent: Thread-1 Jill
    Child: pool-1-thread-2 Jill = Jack
    Parent: Thread-4 Fred
    Child: pool-1-thread-1 Fred = Steve
    Parent: Thread-2 James
    Child: pool-1-thread-2 James = Jack
    The basic workaround for this problem is to do the following:
    Code:
    public class ChildThread implements Runnable {
    
        private final String mUser;
    
        public ChildThread(String user) {
            mUser = user;
        }
    
        @Override
        public void run() {
            try {
                Object user = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
                System.out.println("Child: " + Thread.currentThread().getName() + " " + mUser + " = " + user);
            } finally {
                SecurityContextHolder.clearContext();
            }
        }
    }
    Last edited by james_a_woods; Nov 11th, 2009, 12:13 PM. Reason: Cut and paste error with the code.

  • #2
    On further investigation the workaround seems to cause other problems which I will look at tomorrow. But the basic problem still exists.

    Comment


    • #3
      This sounds like the expected behaviour for InheritableThreadLocal. From the Javadoc:

      When a child thread is created, the child receives initial values for all inheritable thread-local variables for which the parent has values
      Hence the value from the parent thread will only be set when the thread is created. If you retain it in a pool for reuse, then the value will not change unless you change it. You seem to want the context of the calling thread (which invokes the executor) to be used instead of the context of the creating thread. This isn't the same as using an inherited thread-local, so that's not the way to go. You would be better to set the context explicitly yourself, or customize the executor implementation to do so.

      It is generally good practice to clear thread-local variables when returning them to a thread pool, so a try/finally block is always a good idea.

      Comment


      • #4
        Ah, I see what is happening here. I was getting confused as to exactly when the threads in the thread pool where being created and which thread was the parent of the new thread in the pool. So the context needs to be set and cleared for each thread invocation.

        At a primitive level:
        Code:
        public class ChildThread implements Runnable {
        
            private final String mUser;
        
            public ChildThread(String user) {
                mUser = user;
            }
        
            @Override
            public void run() {
                try {
                    SecurityContext ctx = new SecurityContextImpl();
                    ctx.setAuthentication(new UsernamePasswordAuthenticationToken(mUser, null));
                    SecurityContextHolder.setContext(ctx);
        
                    Object user = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
                    System.out.println("Child: " + Thread.currentThread().getName() + " " + mUser + " = " + user);
                } catch (Exception e) {
                    e.printStackTrace();
                } finally {
                    SecurityContextHolder.clearContext();
                }
            }
        }
        Providing a ThreadFactory that does this automatically would be a more elegant solution.

        Section 5.2 of the reference manual gave me a false sense of security when it said:
        Using a ThreadLocal in this way is quite safe if care is taken to clear the thread after the present principal's request is processed. Of course, Spring Security takes care of this for you automatically so there is noneed to worry about it.
        I can see now that what it says is in fact correct. But maybe adding a short sentence warning us to be more careful with thread pools would be helpful for those of us familiarising ourselves with the details of thread pooling .

        Comment

        Working...
        X