0

Lets say I have a code like this in my servlet:

private static final String RESOURCE_URL_PATTERN = "resourceUrlPattern";
private static final String PARAM_SEPARATOR = "|";

private List<String> resourcePatterns;

@Override
public void init() throws ServletException {
    String resourcePatterns = getInitParameter(RESOURCE_URL_PATTERN);
    this.resourcePatterns = com.google.common.base.Splitter.on(PARAM_SEPARATOR).trimResults().splitToList(resourcePatterns);
}

Is this thread safe to use 'resourcePatterns' if it will never be modified?

Lets say like this:

private boolean isValidRequest(String servletPath) {
    for (String resourcePattern : resourcePatterns) {
        if (servletPath.matches(resourcePattern)) {
            return true;
        }
    }
    return false;
}

Should I use CopyOnWriteArrayList or ArrayList is OK in this case?

Slava Vedenin
  • 58,326
  • 13
  • 40
  • 59
Yurii Bondarenko
  • 3,460
  • 6
  • 28
  • 47
  • Will the init() method will be called and completed before any call to isValidRequest ? Also I think you may have some memory issues as your list is not at least volatile – PNS Jan 31 '16 at 22:31
  • @PNS Interesting point. init() is called before. But 'resourcePatterns' is not volatile. It makes me wonder - should I add volatile or it's an overhead? – Yurii Bondarenko Jan 31 '16 at 22:41
  • Too long for comment, srooy, read my answer below :) – PNS Jan 31 '16 at 22:47

2 Answers2

2

Yes, List is fine to read from multiple threads concurrently, so long as nothing's writing.

For more detailed information on this, please see this answer that explains this further. There are some important gotchas.

Community
  • 1
  • 1
DominicEU
  • 3,585
  • 2
  • 21
  • 32
1

From java concurrency in practice we have:

To publish an object safely, both the reference to the object and the object's state must be made visible to other threads at the same time. A properly constructed object can be safely published by:

Initializing an object reference from a static initializer. Storing a reference to it into a volatile field. Storing a reference to it into a final field. Storing a reference to it into a field that is properly guarded by a (synchronized) lock.

your list is neither of these. I suggest making it final as this will make your object effectively immutable which in this case would be enough. If init() is called several times you should make it volatile instead. With this I of course assume that NO changes to the element of the list occur and that you don't expose any elements of the list either (as in a getElementAtPosition(int pos) method or the like.

PNS
  • 750
  • 1
  • 5
  • 19
  • since it's a servlet I can not make 'resourcePatterns' a final field. But what if I make init() method synchonized? According to doc that you provided it should work. – Yurii Bondarenko Jan 31 '16 at 22:56
  • 1
    Well, that would work. But then you really should go for the volatile instead. This is exactly the kind of case where that is usable. You have a field that does not have any synchronization requirements other than with memory. – PNS Jan 31 '16 at 22:58