DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Locks in try-with-resource #1608

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

Hi,

With java 7 we got try-with-resource semantics.
Unfortunately, it does not work with locks and we
still need to write 

 lock.lock();
 try {
  //do some staff
 } finally {
    lock.unlock();
 }

and it's quite often, finally block is forgotten.

There is a simple solution to that which nicely fits into guava

Lock lock = ...;
try (AutoLock l = withLock(lock)) {
     // access the locked resource
}

The class including test attached.

-Kofemann.

Original issue reported on code.google.com by kofem...@gmail.com on 11 Dec 2013 at 11:10

Attachments:

GoogleCodeExporter commented 9 years ago
OT: I am very curious about Guava team's response. According to links at 
http://stackoverflow.com/questions/16574353/any-risk-in-a-autocloseable-wrapper-
for-java-util-concurrent-locks-lock , it seems the suggestion for adding this 
feature to Java was rejected. 

Original comment by tomas.za...@intelis.cz on 11 Dec 2013 at 3:35

GoogleCodeExporter commented 9 years ago
FYI, Guava is still a JDK6 library and can't use AutoCloseable yet.

There's been some debate about this internally, which I'll try to sum up here:

1. Try-with-resources was designed to solve resource management, for example 
opening and closing OutputStreams. This is actually VERY HARD (and verbose) to 
do correctly without try-with-resources. By comparison, acquiring and releasing 
locks is very, very simple. Releasing a lock cannot throw an exception, unlike 
closing an OutputStream.

2. Because of what try-with-resources was designed for, it's extremely awkward 
to use for things like locks and "scoping" blocks to certain contexts (for 
example, a database transaction), because you're required to assign something 
to a variable in the resource declaration part of the try-with-resources block, 
but in neither of those cases do you actually need to USE the variable you 
create. This leads to both unused variable warnings as well as special 
try-with-resources warnings for unused resources.

3. Despite that, people do seem to REALLY want to use try-with-resources with 
locks. And while it's unfortunately awkward to do, it does seem mostly 
harmless. Particularly since it's actually possible to do it in a way that 
doesn't create a new object each time you acquire a lock*.

Personally, while I would have been happy to recommend doing this had 
try-with-resources been designed in a way that intentionally supported this 
type of use, I don't really recommend it given the reality of the situation. I 
don't want to strongly discourage it either... that seems like a bit of a 
losing battle from what I can tell. I would suggest implementing it in a way 
that avoids object creation though.

* Something like this:

class FooLock implements Lock {
  private final Lock delegate;
  private final Unlocker unlocker; // implements AutoCloseable

  FooLock(Lock delegate) {
    this.delegate = delegate;
    this.unlocker = new Unlocker(delegate);
  }

  public Unlocker acquireLock() {
    lock.lock();
    return unlocker;
  }

  // ... other methods
}

Original comment by cgdecker@google.com on 11 Dec 2013 at 5:51

GoogleCodeExporter commented 9 years ago

Just want to comment on point 3.

I believe, the main motivation is simple syntax which we already have with 
synchronized:

synchronized(this){
  ...
}

It's natural, with the language which supports block to release resources 
allocated when leaving the block.

It's clear, that you shold not add into guava any crap ( that's the reason why 
we love it). So feel free to close this issue.

Original comment by kofem...@gmail.com on 11 Dec 2013 at 7:41

GoogleCodeExporter commented 9 years ago
I agree that it's natural to want to be able to something similar to a 
synchronized block with locks... the problem being try-with-resources just 
doesn't really support that, no matter how much one might want it to.

I'm not sure I want to close this issue just yet. It's not something we can do 
in Guava at the moment as a JDK6 library, but I don't want to rule it out 
entirely, since given the huge desire to do this it *might* be preferable to 
create one good implementation in Guava. Maybe.

Original comment by cgdecker@google.com on 11 Dec 2013 at 8:05

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08