Working on reentrant lock migration

[jbescos]

#128

[jmehrens]

The synchronized here was just paranoia so that other threads couldn't split the head/format/tail. If the developer did something foolish like:

Handler first = ....
Handler second = ...
second.setFormatter(new CollectorFormatter(null, first.getFormatter(), null));
logger.addHandler(first);
logger.addHandler(second);

I would just eliminate the flock and if anyone reports a problem we'll just tell them to not let the target formatter escape collectorformatter.

[jbescos]

I didn't want to make any improvement in this PR rather than migrating to ReentrantLock because, in case I introduce a new bug, it will be more difficult to spot it.

It makes more sense for me to create a new PR with improvements, and then I can try to migrate to ReentrantLock here.

In this way, if there is a bug, we can try to reproduce it with 2 versions to know whether it is caused by ReentrantLock PR or synchronization improvements PR.

[jmehrens]

That sounds reasonable. However there is not an equivalent transformation because synchronized(f) is acquiring the lock of 'f'. So if you are going more compatible, the get rid of 'fLock' and check if 'f' is a CollectorFormatter and aquire f.lock.lock() If it is not a CollectorFormatter, synchronize or don't. No fix for this case because 'f' can be anything.

[jmehrens]

I think one option would be:

1. Remove this.fLock.
2. Use a local 'fLock' and assign it f.lock if f is a CollectorFormatter or our lock if not.
3. Returning our lock is a minor waste but, keeps the code a little cleaner around lock and unlock.

       if (f != null) {
            ReentrantLock flock = f instanceof CollectorFormatter 
                                    ?  ((CollectorFormatter) f).lock : lock;
            fLock.lock();
            try {
                head = f.getHead(h);
                msg = record != null ? f.format(record) : "";
                tail = f.getTail(h);
            } finally {
                fLock.unlock();
            }
      } else {

[jbescos]

I am going to wait to see what happens with the synchronized blocks in JDK21+.