StringBuffer or synchronized method?

Question

I have following code:

public class MyLogger {
   private StringBuilder logger = new StringBuilder();

   public void log(String message, String user) {
      logger.append(message);
      logger.append(user);
   }
}

The programmer must guarantee that a single MyLogger object works properly for a multi-threaded system. How must this code be changed to be thread-safe?

A. synchronize the log method

B. replace StringBuilder with StringBuffer

Please advise the best approach.

I would probably not use either, preferring a 'LogEntry' class instance that holds a copy of both strings and a command to a logger thread that waits at the end of a Blocking Queue. — Martin James, Aug 18 '17 at 19:18

score 3 · Accepted Answer · edited Jun 20 '20 at 09:12

3

StringBuffer is not enough to guarantee that the two information are always sent grouped :

  logger.append(message);
  logger.append(user);

It guarantees only that append() will not be invoked in a concurrent way and that so you don't risk to break the state of the StringBuffer object.
You could indeed have two threads that interleave during the log() invocation and get these invocations :

Thread 1 : logger.append(message);

Thread 2 : logger.append(message);

Thread 1 : logger.append(user);

Thread 2 : logger.append(user);

This is so better :

  public synchronized void log(String message, String user) {
      logger.append(message);
      logger.append(user);
   }

You can also do the synchronization on the private logger object.
Which has the advantage to not allow the client of the class to lock it outside the class.

edited Jun 20 '20 at 09:12

Community

1
1

answered Aug 18 '17 at 18:47

davidxxx

125,838
23
214
215

2

You may want to mention that synchronizing on a private object is a better choice than using `synchronized` on the method signature. – Alvin Thompson Aug 18 '17 at 18:50
1

@Alvin Thompson nice advise. I added it. – davidxxx Aug 18 '17 at 18:56
Thanks @davidxxx for the nice description. Please let me know if my understandng is wrong: say,Thread 1 got the chance to perform logger.append(message); so T1 has the lock on SB object and on completion of the method execution it will release lock on SB? so T2 can perform logger.append(message); and thus problem may arise? is this what you wanted to describe? – Abanti Chattopadhyay Aug 21 '17 at 13:26
you are welcome. You are close, "and on completion of the method execution it **may** release lock on". – davidxxx Aug 21 '17 at 14:12
okay. Hence problem arises when T1 releases the lock and T2 may start performing, i.e., Thread 1 : logger.append(message); Thread 2 : logger.append(message); Thread 1 : logger.append(user); Thread 2 : logger.append(user); otherwise if T1 holds the lock then it can continue with: logger.append(user); and T2 will not get a chance, so this will be like: Thread 1 : logger.append(message); Thread 1 : logger.append(user); Thread 2 : logger.append(message); Thread 2 : logger.append(user); – Abanti Chattopadhyay Aug 22 '17 at 04:23
you got it. It is not deterministic. Without synchronizing the method, even if T1 wrote the message first, you may "randomly" finish in the first or the second scenario that you described. – davidxxx Aug 22 '17 at 13:08

score 0 · Answer 2 · answered Aug 18 '17 at 18:50

Use A, because if you choose to use StringBuffer you could mess the log: This could happen with StringBuffer (will generate wrong log):

logger.append(message); < Thread A
logger.append(message); < Thread B
logger.append(user);  < Thread A
logger.append(user); < Thread B

StringBuffer or synchronized method?

2 Answers2