5

I am working on Webmethods Integration Server. Inside there is a java service which is in form of a static java method for writing data to a log file (server.log) by using BufferedWriter and FileWriter. The static method code is like this:

public static void writeLogFile(String message) throws ServiceException{
    try {
        BufferedWriter bw = new BufferedWriter(new FileWriter("./logs/server.log", true));
        bw.write(message);
        bw.newLine();
        bw.close();
    } catch (Exception e) {
        throw new ServiceException(e.getMessage());
    }
}

Note:
-The code has been simplified for example purpose.
-I can't change the writeLogFile method declaration and attribute. That means, it will always be: public static void writeLogFile. This kind of modification is prohibited: public synchronized void writeLogFile.

There is a chance that the writeLogFile method can be invoked by different instances, so I need to make sure that there are no two or more instances access same resource (server.log) in same time. That means, if there are two instances try to access the server.log, one of the instances must have to wait another instance to finish writing data to the server.log.

The questions are: Should I change the code above? If so, what kind of modification I need to do? Should I implement "synchronized" inside the java static method?

@EJP:
So, which one below is the best code to implement synchronized?

1)

        FileWriter fw = new FileWriter("./logs/server.log", true);
        synchronized (fw) {
            BufferedWriter bw = new BufferedWriter(fw);
            bw.write(message);
            bw.newLine();
            bw.close();
        }

2)

        BufferedWriter bw = new BufferedWriter(new FileWriter("./logs/server.log", true));
        synchronized(bw) {
            bw.write(message);
            bw.newLine();
            bw.close();
        }

3)

        synchronized(util.class) {  //yes, the class name is started with lowercase
            BufferedWriter bw = new BufferedWriter(new FileWriter("./logs/server.log", true));
            bw.write(message);
            bw.newLine();
            bw.close();
        }

4) Other opinion?

Thanks.

null
  • 8,669
  • 16
  • 68
  • 98

3 Answers3

2

Just make the method synchronized. It doesn't affect its method signature for binary compatibility purposes.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • I can't do it because I can't change the method declaration too. It's a fixed rule from webmethods, it always be: public static void xxx – null Sep 29 '11 at 05:52
  • @suud: `synchronized` doesn't need to be declared. You can put it into the definition only. – Joonas Pulakka Sep 29 '11 at 06:09
  • So put a synchronized (XXX.class) block around the code inside. But personally I would try to keep the writer open. – user207421 Sep 29 '11 at 06:19
  • @EJP: please look at the original post, I made a comment over there. – null Sep 29 '11 at 07:03
  • @suud I've answered all that. I've told you to synchronize on the class literal, and, as I've told swanliu, the method creates instances of both Writers per invocation, so synchronizing on those is futile. – user207421 Sep 29 '11 at 08:20
  • @EJP: ok. Let's say if I can add "synchronized" keyword to the method (so it become: public synchronized static void writeLogFile), does it better than using synchronized(xxx.class) ? – null Sep 29 '11 at 08:25
  • It's exactly the same. I prefer adding it to the signature as it's less code. – user207421 Sep 29 '11 at 08:28
  • @EJP: wait, I need clarification. Syncrhonizing BufferedWriter & FileWriter is futile because they're always created as new instance inside the code, but isn't it same case for xxx.class? For example, there are two webmethod servers which have their own xxx.class. Doesn't it mean each xxx.class is different for both server? – null Sep 29 '11 at 08:41
  • @suud If this piece of code is going to be executed in multiple JVMs, let alone multiple servers, no amount of synchronization is going to help: what you need is a database. – user207421 Sep 29 '11 at 09:59
0

I have another suggestion. I guess synchronization can be treated as an aspect and the same can be achieved using some AOP framework. This conforms to you requirement of not changing the code. But I am not 100% sure about it and posted a question for the same. Please monitor its responses .

Community
  • 1
  • 1
Santosh
  • 17,667
  • 4
  • 54
  • 79
-1

No. the base class of BufferedWriter and FileWriter is java.io.Writer,

it has it own lock on each write operation 

Object java.io.Writer.lock

The object used to synchronize operations on this stream.

try make the BufferedWriter bw static and ref to it by a static method ,thus all write is write to file via same Writer object

btw, i guess you are inventing yet-another-log-lib... may be you could use log4j or any kind of log lib instead

swanliu
  • 781
  • 3
  • 8
  • No. That's an instance member. Each caller of this method will get his own instance of FileWriter and BufferedWriter. – user207421 Sep 29 '11 at 05:29
  • I don't use log4j, because log4j doesn't support this kind of scenario: writing a appender log file which the output log filename has a pattern: moduleName_currentDate.log (the currentDate's pattern is yyyyMMdd). – null Sep 29 '11 at 06:01
  • log4j support create FileAppender in runtime. check parameterized constructor of FileAppender http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/FileAppender.html – swanliu Sep 29 '11 at 06:14
  • am i misunderstood question? or you can make the BufferedWriter bw static and ref to it by a static method thus all write is write to file via same writer object – swanliu Sep 29 '11 at 06:17
  • You can rewrite the code entirely, and I would, but your answer as applied to the OP's code is incorrect. You are also overlooking the effect of buffering, and conversely of the fact that he is calling write() and newline() separately, which introduces a timing window. – user207421 Sep 29 '11 at 06:18
  • @swanliu: yes, I think it can, but still I have to write my own logic to define the output filename pattern (moduleName_currentDate.log), and some chores must be worked on it, like writing the code to read the log4j configuration file, etc. I think using built-in java I/O class is much faster and simpler. – null Sep 29 '11 at 07:13