#11 Logging mechanism is not thread safe

[David Newcomb]

public class Log implements Runnable {
/** It store data collected by elvyx */
Data data = null;
/** List of data */
private static List dataList = Collections.synchronizedList(new ArrayList());
/** Flag */
private static boolean flag = false;

public Log(String connectionId, String initDate, String initExecution, String endExecution,
String category, String sql, String preparedSql) {
data = new Data(connectionId, initDate, initExecution, endExecution, category, sql, preparedSql);
}

public void run() {
// Try to send it else, save the information on a flat file
dataList.add(data);
if (!flag) {
flag = true;
try {
FlatFile.save(dataList);
send();

Additions to dataList is synchronised but the object is static which means it is shared across all instances of this class. The creating of "statement.dat" is not synchronised so different threads interfere and overwrite the file as it is being created. When load is higher almost non of the files are created successfully, and so the client gets rubbish.

Short term sugggestions:
1. dataList should not be static and as a consequence you don't need the overhead of a sync list.
2. statement.dat should include thread id of avoid conflicts.

Longer term suggestion:
Even though you have a thread pool to do the sending you the overhead of zipping and sending a file over the (local) LAN which is extememly heavy.
My application does *a lot* concurrent queries and this method is too heavy.

class Data implements Serializable
{...

class DataSender extends Thread
{
private List toSend ;
private static Object lock ;

DataSender ()
{
// We want it die when the program terminate
setDeamon(true);
toSend = new ArrayList(Config.senderLen);

start();
}

public void run()
{
Data[] transport;
while (true)
{
try
{
synchronized (this)
{
wait(Config.waitTime);
}

if (toSend.size() == 0)
continue;

synchronized (lock)
{
// Convert to transport type
transport = toSend.toArray()
// flatten the list
toSend = new ArrayList(Config.senderLen);
}
doSend(transport );
}
catch (Exception e)
{
}
}
}

public static void queue(Data d)
{
syncronized (lock)
{
toSend.add(d);
}
}
private void doSend(Data[] transport)
{
convert to stream and send over the wire
}
}

Data objects can still be queued up while a send is in progress.
You can extend DataSender to send XML, stream, etc.. by overloading the doSend method.
You can also define your own policy regarding when things are set ie every 100 items or every 10 seconds.
The thread pool is not really needed.

There is no mailing list set up on SF for discussion.