Review Locker

[sbordet]

Class Locker has an _unlock field that is completely unnecessary, wasting memory and causing pointer chasing to call close() on it.
Class Locker.Lock is not static so it has a hidden field pointing back to Locker increasing memory waste.

I propose the introduction of a new class called AutoLock which implements AutoCloseable and returns itself:

class AutoLock implements AutoCloseable
{
    private final ReentrantLock _lock = new ReentrantLock();

    public AutoLock lock()
    {
        _lock.lock();
        return this;
    }

    @Override
    public void close()
    {
        _lock.unlock();
    }

    ...
}

The idiom will change from:

try (Locker.Lock lock = _locker.lock())
{
}

to

try (AutoLock lock = _autoLock.lock())
{
}

This will require a large change in the codebase, but has the advantage that it will be as easy to use as synchronized and will play better for the future in case of Project Loom.

[gregw]

@sbordet You'll have to also support newCondition() API, but I think that is trivial.

We can remove the lockIfNotHeld stuff as no longer used.

@janbartel I think we should use this issue and resulting PR to cleanup the usage of Locker.isLocked() by sessions. I think that was introduced as a defence when the locking was last changed, but I don't think we need to check it anymore. Note there are a few methods that do call session methods without the lock (and thus will fail this test), but they are deprecated and/or never called - so we can just fail in them rather than check everywhere.