这个锁管理代码片段是否过于复杂?

Is this lock management code snippet unnecessarily complicated?

Hewlett Packard Enterprise 应用程序 Fortify On Demand 包含标题为 "Example 2: The following code will always release the lock."

的代码片段
ReentrantLock  myLock = new ReentrantLock();

try {
    myLock.lock();
    performOperationInCriticalSection();
    myLock.unlock();
}
finally {
    if (myLock != null) {
        myLock.unlock();
    }
}

为什么 try 块中的最后一行解锁锁,而它已经在 finally 中处理过?它真的有必要,还是只是一些冗长的编码标准的一部分? if 看起来也没有必要。

那个代码throws an IllegalMonitorStateException,因为它连续调用了两次unlock()

来自 ReentrantLock.unlock:

Throws:

IllegalMonitorStateException - if the current thread does not hold this lock

我不得不说代码不正确,我真的猜不出他们想做什么。如果在 try 块内的 unlock() 之后有一个 myLock = null; ,它可能会起作用,但这样做仍然是草率的代码。如果 unlock() 抛出异常,那么 unlock() 的第二次调用也会抛出另一个异常。

根据文档,正确的成语是这样的,例如Lock:

Lock l = ...;
l.lock();
try {
    // access the resource protected by this lock
} finally {
    l.unlock();
}

如果你想要真正安全,你可以使用 try-with-resources 来利用 if close() 抛出异常而异常从 try 块被抛出, close() 异常被添加到被抑制的异常中,因此它们都不会丢失。 (在前面的示例和 HP 示例中,如果 unlock() 抛出异常,则从 try 块抛出的异常将完全丢失。)

class Locker implements AutoCloseable {
    private final Lock lock;
    Locker(Lock lock) { this.lock = Objects.requireNonNull(lock);
                        this.lock.lock(); }
    @Override public void close() { lock.unlock(); }
}

Lock lock = ...;
try (Locker locker = new Locker(lock)) {
    // If both modifySharedState() and unlock()
    // throw an exception, the unlock() exception
    // is added to the modifySharedState()
    // exception's suppressed list.
    modifySharedState();
}

(Here's an example output using that code on Ideone.)

或者你可以写类似代码的东西 which try-with-resources translates to:

Lock lock = ...;
lock.lock();
Throwable primary = null;
try {
    modifySharedState();
} catch (Throwable x) {
    primary = x;
    throw x;
} finally {
    try {
        lock.unlock();
    } catch (Throwable x) {
        if (primary != null) {
            primary.addSuppressed(x);
            // primary is already "in-flight"
            // so no need to throw it again
        } else {
            throw x;
        }
    }
}

虽然这有点难以理解。

如果逻辑比 lock()...unlock() 更复杂,考虑 unlock() 抛出异常可能是有意义的,例如,如果您有多个锁或 unlock() 是有条件的出于某种原因。