Hi!

I can't see how this function can be thread safe.
We've encountered thread problems in our code when
we added high test loads, and they went away when
I removed the SocketAppender.

The function basically does this, minus some error
handling and other issues irrelevant for thread
safety:

public
void append(LoggingEvent event) {
    // ... removed stuff ...
    oos.writeObject(event);
    //LogLog.debug("=========Flushing.");
    oos.flush();
    if(++counter >= RESET_FREQUENCY) {
      counter = 0;
      // Failing to reset the object output stream every
      // now and then creates a serious memory leak.
      //System.err.println("Doing oos.reset()");
      oos.reset();
    }
}

If two threads in my program logs at the same time,
then this function might be called in parallel. One
thread could be interrupted between writeObject()
and flush() (or even at the middle of the writeObject()
function), at which time the other thread could
step in and mess it all up. It gets even worse if one
thread calls reset() while the other is in the middle
of writeObject().

Or am I missing something?

Shouldn't it be:

public
void append(LoggingEvent event) {
    // ... removed stuff ...
    synchronized (oos) {                          // NEW
      oos.writeObject(event);
      //LogLog.debug("=========Flushing.");
      oos.flush();
      if(++counter >= RESET_FREQUENCY) {
        counter = 0;
        // Failing to reset the object output stream every
        // now and then creates a serious memory leak.
        //System.err.println("Doing oos.reset()");
        oos.reset();
      }
    }
}

This would, of course, reduce the overall performance, but
I can't see any other way to do it.

Mats

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to