Java volatile and/or synchronized
concurrency, java, synchronized, volatile
Solution
There are multiple issues with this code.
Never use a timestamp as UID, unless you're absolutely positive, there won't be ever generated multiple UIDs within a time that is smaller than the lowest resolution of the timestamp you're using. I'd recommend switching to a completely different approach. Either append a counter to the timestamp, if you absolutely want to keep the timestamp-format in use or simply use a counter. Another alternative would be to use System.nanoTime() in addition to the normal systemtime, though that approach might provide quite a few pitfalls.
Your while-loop will loop for up to an entire millisecond, if you try to generate two UIDs within the same millisecond. There's no fast computer needed to make this a total waste of CPU-time. The loop will at least run several thousand times without proper result.
Marking a variable `volatile` won't do. You have to mark the entire block that is run within the method `synchronized` to prevent multiple threads from running it at the same time. But consider a case, where you want to generate 1000 UIDs within a single ms. What should be done within no time now suddenly takes a full second. You're creating an enormous bottleneck.
My recommendation: Delete that method immediately. There's not much that could fix this code to the point where it would actually be acceptable in terms of performance and correctness. Read this tutorial about concurrency. Get a new approach for generating UIDs and start over from scratch.
Alternatively: Why even write code for something that already exists? Use the UID-class provided by Oracle. Another good approach would be to use UUID, which is part of the utility package and quite likely more general than `UID`. Depends on your demands on the generated UID.
Problem
I have a static method that is supposed to generate a unique ID based on the current timestamp as shown in the codes below. To ensure that the newly generated ID is not the same as the previously generated ID (due to very fast computer such that the millisecond does not change), I put in a loop to compare the newly generated ID against the previously generated one. If they are the same, it will generate another ID. ``` public class Util { protected static String uniqueID; public static String generateUniqueID() { SimpleDateFormat timstampFormat = new SimpleDateFormat("yyyyMMddHHmmssSSS"); do { String timestamp = timstampFormat.format(new Date()); if (!timestamp.equals(uniqueID)) { uniqueID = timestamp; return uniqueID; } } while (true); } } ``` I want the above codes to work when the method is called by multiple threads. If I merely put the volatile keyword to the uniqueID variable, would that be good enough? Do I still need to have a synchronized block? How about having a synchronized block but without the volatile keyword? Thanks in advance. ADDED: If I changed to below codes, would the volatile keyword still required? ``` public class Util { private static volatile String uniqueID; public static synchronized String generateUniqueID() { uniqueID = UUID.randomUUID().toString(); return uniqueID; } } ```