CODEWITHSUNDEEP
javaconcurrencysemaphore
Joonhyeok Im
Joonhyeok Im

Reputation: 23

Semaphore manipulation in finally clause

I am reading Java Concurrency In Practice.

public class BoundedHashSet<T> {
  private final Set<T> set;
  private final Semaphore sem;

  public BoundedHashSet(int bound) {
    this.set = Collections.synchronizedSet(new HashSet<T>());
    sem = new Semaphore(bound);
  }

  public boolean add(T o) throws InterruptedException {
    sem.acquire();
    boolean wasAdded = false;
    try {
      wasAdded = set.add(o);
      return wasAdded;
    } finally {
      if (!wasAdded)
        sem.release();
    }
  }

  public boolean remove(Object o) {
    boolean wasRemoved = set.remove(o);
    if (wasRemoved)
      sem.release();
    return wasRemoved;
  }
}

I don't know why the add method has try-finally clause and the remove method does not. I think it is ok to make it simpler like the following.

public boolean add(T o) throws InterruptedException {
   sem.acquire();
   boolean wasAdded = set.add(o);
   if (!wasAdded)
       sem.release();
   return wasAdded;
}

What is the benefit of placing try-finally instead of simple statements?

Upvotes: 2

Views: 121

Answers (1)

Slaw
Slaw

Reputation: 45806

TL:DR: The call to Collection#add(E) could throw an exception after acquiring a permit.

The permits of the Semaphore in this case represent the capacity. When an element is added a permit is acquired and then there's an attempt to add the element to the internal Set. The result of calling add is then checked to make sure the element was actually added. If the element was not added then the permit that was previously acquired must be released. All good so far.

The issue is that Collection#add(E) can throw an exception. The acquired permit must still be released in that case. The try-finally block guarantees that if the element was not added, either because duplicates are not allowed or because an exception was thrown, the previously acquired permit is released.

public boolean add(T o) throws InterruptedException {
  sem.acquire(); // permit acquired before attempting to add element
  boolean wasAdded = false;
  try {
    wasAdded = set.add(o);
    return wasAdded;
  } finally {
    if (!wasAdded)
      sem.release(); // invoked if "set.add(o)" returns false **or** throws exception
  }
}

Since your version does not have a try-finally block the code is broken:

public boolean add(T o) throws InterruptedException {
   sem.acquire(); // permit acquired before attempting to add element
   boolean wasAdded = set.add(o); // if exception is thrown permit is never released
   if (!wasAdded)
       sem.release(); // only invoked if "set.add(o)" returns false
   return wasAdded;
}

Removing an element does not have the same problem. No permit was released before attempting to remove the element, thus no permit has to be re-acquired if the element was not actually removed. This is still the case even if Collection#remove(Object) throws an exception.

public boolean remove(Object o) {
  boolean wasRemoved = set.remove(o);
  if (wasRemoved)
    sem.release(); // won't be invoked if "set.remove(o)" returns false or throws exception
  return wasRemoved;
}

Upvotes: 3

Related Questions