Is it possible for code like this to be not thread safe? It is a static method and we are using an local instance of stringbuilder. I guess the input strings might be held by other objects?
public static String cat(final String ... strings) {
...
...
final StringBuilder sb = new StringBuilder(totLength);
for (int i = 0; i < size; i++) {
if (strings[i] != null) {
sb.append(strings[i]);
}
}
return sb.toString();
}
That should be Thread Safe, as the Strings being passed in are immutable. and assuming you create totLength within the method, everything else is local .
EDIT:
as Jon Skeet points out, there is a chance that the vargs value could be passed in not only as a sequence of Strings (as my answer assumes), but also as a String[]. In the latter case, there is the possibility for the array to be modified by another thread while you are processing.
No, it's thread-safe as written(*). Strings are immutable, so it doesn't matter if multiple threads call this.
(* for the code you show)
This would also be threadsafe even if you weren't passing in immutable objects, because
See John Skeet's answer for the array part.
As for the StringBuilder, it should be safe. Because the StringBuilder is created inside the method, each call has its own StringBuilder.
StringBuilder itself is not synchronized, so if it existed before the static method call, you'd be better off using a StringBuffer instead.
It's not fully thread safe - because another thread might be changing the same array that was passed in the argument for the strings parameter. This isn't entirely obvious because of your use of varargs, but effectively (in terms of thread safety) the method signature is just:
public static String cat(String[] strings)
It won't cause any exceptions, but you may not see the latest values within the array.
As another alternative, you might see something unexpected if it does see the change. For example, suppose we pass in just a single-valued array where the value is initially "x":
public static String cat(final String ... strings) {
...
...
final StringBuilder sb = new StringBuilder(totLength);
for (int i = 0; i < size; i++) {
// strings[0] is currently "x"
if (strings[i] != null) {
// But now another thread might change it to null!
// At that point we'd get "null" as output
sb.append(strings[i]);
}
}
return sb.toString();
}
In other words, although you'd probably expect to either see "x" or "" as the result, you could see "null". To fix that, you can read each value from the array just once:
final StringBuilder sb = new StringBuilder(totLength);
for (int i = 0; i < size; i++) {
String value = strings[i];
if (value != null) {
sb.append(value);
}
}
You may still see an array which is half way through being changed (e.g. if one thread is changing {"x", "y"} to {"a", "b"} you may see "xb" as the result), but you won't get a spurious "null".