Add shorthands for locking the session (#10225)

Change-Id: Ic2f3843a48a6e26af848208e9495473853146fbe

7 files changed, 84 insertions, 36 deletions

diff --git a/server/src/com/vaadin/server/AbstractCommunicationManager.java b/server/src/com/vaadin/server/AbstractCommunicationManager.java
index 731d83067e..31920b5df6 100644
--- a/server/src/com/vaadin/server/AbstractCommunicationManager.java
+++ b/server/src/com/vaadin/server/AbstractCommunicationManager.java
@@ -297,12 +297,12 @@ public abstract class AbstractCommunicationManager implements Serializable {
                 cleanStreamVariable(owner, variableName);
             }
         } catch (Exception e) {
-            session.getLock().lock();
+            session.lock();
             try {
                 handleChangeVariablesError(session, (Component) owner, e,
                         new HashMap<String, Object>());
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
         }
         sendUploadResponse(request, response);
@@ -345,12 +345,12 @@ public abstract class AbstractCommunicationManager implements Serializable {
                 cleanStreamVariable(owner, variableName);
             }
         } catch (Exception e) {
-            session.getLock().lock();
+            session.lock();
             try {
                 handleChangeVariablesError(session, (Component) owner, e,
                         new HashMap<String, Object>());
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
         }
         sendUploadResponse(request, response);
@@ -382,13 +382,13 @@ public abstract class AbstractCommunicationManager implements Serializable {
                 filename, type, contentLength);
         try {
             boolean listenProgress;
-            session.getLock().lock();
+            session.lock();
             try {
                 streamVariable.streamingStarted(startedEvent);
                 out = streamVariable.getOutputStream();
                 listenProgress = streamVariable.listenProgress();
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
 
             // Gets the output target stream
@@ -409,13 +409,13 @@ public abstract class AbstractCommunicationManager implements Serializable {
                 if (listenProgress) {
                     // update progress if listener set and contentLength
                     // received
-                    session.getLock().lock();
+                    session.lock();
                     try {
                         StreamingProgressEventImpl progressEvent = new StreamingProgressEventImpl(
                                 filename, type, contentLength, totalBytes);
                         streamVariable.onProgress(progressEvent);
                     } finally {
-                        session.getLock().unlock();
+                        session.unlock();
                     }
                 }
                 if (streamVariable.isInterrupted()) {
@@ -427,11 +427,11 @@ public abstract class AbstractCommunicationManager implements Serializable {
             out.close();
             StreamingEndEvent event = new StreamingEndEventImpl(filename, type,
                     totalBytes);
-            session.getLock().lock();
+            session.lock();
             try {
                 streamVariable.streamingFinished(event);
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
 
         } catch (UploadInterruptedException e) {
@@ -439,17 +439,17 @@ public abstract class AbstractCommunicationManager implements Serializable {
             tryToCloseStream(out);
             StreamingErrorEvent event = new StreamingErrorEventImpl(filename,
                     type, contentLength, totalBytes, e);
-            session.getLock().lock();
+            session.lock();
             try {
                 streamVariable.streamingFailed(event);
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
             // Note, we are not throwing interrupted exception forward as it is
             // not a terminal level error like all other exception.
         } catch (final Exception e) {
             tryToCloseStream(out);
-            session.getLock().lock();
+            session.lock();
             try {
                 StreamingErrorEvent event = new StreamingErrorEventImpl(
                         filename, type, contentLength, totalBytes, e);
@@ -458,7 +458,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
                 // terminalErrorHandler)
                 throw new UploadException(e);
             } finally {
-                session.getLock().unlock();
+                session.unlock();
             }
         }
         return startedEvent.isDisposed();
@@ -571,7 +571,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
         // The rest of the process is synchronized with the session
         // in order to guarantee that no parallel variable handling is
         // made
-        session.getLock().lock();
+        session.lock();
         try {
 
             // Verify that there's an UI
@@ -606,7 +606,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
                     outWriter, uI, analyzeLayouts);
             postPaint(uI);
         } finally {
-            session.getLock().unlock();
+            session.unlock();
         }
 
         outWriter.close();
@@ -2439,7 +2439,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
     public void handleBrowserDetailsRequest(VaadinRequest request,
             VaadinResponse response, VaadinSession session) throws IOException {
 
-        session.getLock().lock();
+        session.lock();
 
         try {
             assert UI.getCurrent() == null;
@@ -2468,7 +2468,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
             // TODO Auto-generated catch block
             e.printStackTrace();
         } finally {
-            session.getLock().unlock();
+            session.unlock();
         }
     }
 
diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index a4caf6cdd6..41115a3503 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -642,14 +642,14 @@ public abstract class VaadinService implements Serializable {
         int uiId = Integer.parseInt(uiIdString);
 
         // Get lock before accessing data in session
-        session.getLock().lock();
+        session.lock();
         try {
             UI ui = session.getUIById(uiId);
 
             UI.setCurrent(ui);
             return ui;
         } finally {
-            session.getLock().unlock();
+            session.unlock();
         }
     }
 
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index 74e3bd360d..3fc75b67f4 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -743,16 +743,64 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
     }
 
     /**
-     * Gets the lock that should be used to synchronize usage of data inside
-     * this session.
+     * Gets the {@link Lock} instance that is used for protecting the data of
+     * this session from concurrent access.
+     * <p>
+     * The <code>Lock</code> can be used to gain more control than what is
+     * available only using {@link #lock()} and {@link #unlock()}. The returned
+     * instance is not guaranteed to support any other features of the
+     * <code>Lock</code> interface than {@link Lock#lock()} and
+     * {@link Lock#unlock()}.
+     * 
+     * @return the <code>Lock</code> that is used for synchronization, never
+     *         <code>null</code>
      * 
-     * @return the lock that should be used for synchronization
+     * @see #lock()
+     * @see Lock
      */
-    public Lock getLock() {
+    public Lock getLockInstance() {
         return lock;
     }
 
     /**
+     * Locks this session to protect its data from concurrent access. Accessing
+     * the UI state from outside the normal request handling should always lock
+     * the session and unlock it when done. To ensure that the lock is always
+     * released, you should typically wrap the code in a <code>try</code> block
+     * and unlock the session in <code>finally</code>:
+     * 
+     * <pre>
+     * session.lock();
+     * try {
+     *     doSomething();
+     * } finally {
+     *     session.unlock();
+     * }
+     * </pre>
+     * <p>
+     * This method will block until the lock can be retrieved.
+     * <p>
+     * {@link #getLockInstance()} can be used if more control over the locking
+     * is required.
+     * 
+     * @see #unlock()
+     * @see #getLockInstance()
+     */
+    public void lock() {
+        getLockInstance().lock();
+    }
+
+    /**
+     * Unlocks this session. This method should always be used in a finally
+     * block after {@link #lock()} to ensure that the lock is always released.
+     * 
+     * @see #unlock()
+     */
+    public void unlock() {
+        getLockInstance().unlock();
+    }
+
+    /**
      * Stores a value in this service session. This can be used to associate
      * data with the current user so that it can be retrieved at a later point
      * from some other part of the application. Setting the value to
diff --git a/uitest/src/com/vaadin/tests/application/ThreadLocalInstances.java b/uitest/src/com/vaadin/tests/application/ThreadLocalInstances.java
index 57c4e3c7b9..82d876d9cb 100644
--- a/uitest/src/com/vaadin/tests/application/ThreadLocalInstances.java
+++ b/uitest/src/com/vaadin/tests/application/ThreadLocalInstances.java
@@ -34,11 +34,11 @@ public class ThreadLocalInstances extends AbstractTestCase {
                 Thread thread = new Thread() {
                     @Override
                     public void run() {
-                        getSession().getLock().lock();
+                        getSession().lock();
                         try {
                             reportCurrentStatus("background thread");
                         } finally {
-                            getSession().getLock().unlock();
+                            getSession().unlock();
                         }
                     }
                 };
diff --git a/uitest/src/com/vaadin/tests/components/table/RowUpdateShouldRetainContextMenu.java b/uitest/src/com/vaadin/tests/components/table/RowUpdateShouldRetainContextMenu.java
index 44fc77d142..bcae4dc6d9 100644
--- a/uitest/src/com/vaadin/tests/components/table/RowUpdateShouldRetainContextMenu.java
+++ b/uitest/src/com/vaadin/tests/components/table/RowUpdateShouldRetainContextMenu.java
@@ -36,11 +36,11 @@ public class RowUpdateShouldRetainContextMenu extends TestBase {
                         sleep(1000);
                     } catch (InterruptedException ie) {
                     }
-                    getContext().getLock().lock();
+                    getContext().lock();
                     try {
                         indicator.setValue(progress += 0.01);
                     } finally {
-                        getContext().getLock().unlock();
+                        getContext().unlock();
                     }
                 }
             }
diff --git a/uitest/src/com/vaadin/tests/components/table/TableFirstRowFlicker.java b/uitest/src/com/vaadin/tests/components/table/TableFirstRowFlicker.java
index 1df2ab0ae1..95d5ea59d5 100644
--- a/uitest/src/com/vaadin/tests/components/table/TableFirstRowFlicker.java
+++ b/uitest/src/com/vaadin/tests/components/table/TableFirstRowFlicker.java
@@ -43,7 +43,7 @@ public class TableFirstRowFlicker extends LegacyApplication {
             @Override
             public void run() {
                 while (t != null) {
-                    t.getUI().getSession().getLock().lock();
+                    t.getUI().getSession().lock();
                     try {
                         int firstId = t.getCurrentPageFirstItemIndex();
                         Object selected = t.getValue();
@@ -53,7 +53,7 @@ public class TableFirstRowFlicker extends LegacyApplication {
                         // lighter alternative for all of above
                         // t.refreshRowCache();
                     } finally {
-                        t.getUI().getSession().getLock().unlock();
+                        t.getUI().getSession().unlock();
                     }
                     try {
                         Thread.sleep(500);
diff --git a/uitest/src/com/vaadin/tests/containers/sqlcontainer/MassInsertMemoryLeakTestApp.java b/uitest/src/com/vaadin/tests/containers/sqlcontainer/MassInsertMemoryLeakTestApp.java
index 8fdc60f293..ebf68fce9a 100644
--- a/uitest/src/com/vaadin/tests/containers/sqlcontainer/MassInsertMemoryLeakTestApp.java
+++ b/uitest/src/com/vaadin/tests/containers/sqlcontainer/MassInsertMemoryLeakTestApp.java
@@ -47,7 +47,7 @@ public class MassInsertMemoryLeakTestApp extends LegacyApplication {
 
         @Override
         public void start() {
-            getContext().getLock().lock();
+            getContext().lock();
             try {
                 proggress.setVisible(true);
                 proggress.setValue(new Float(0));
@@ -56,7 +56,7 @@ public class MassInsertMemoryLeakTestApp extends LegacyApplication {
                 proggress.setCaption("");
                 super.start();
             } finally {
-                getContext().getLock().unlock();
+                getContext().unlock();
             }
         }
 
@@ -78,14 +78,14 @@ public class MassInsertMemoryLeakTestApp extends LegacyApplication {
                                     getRandonName());
                         }
                         c.commit();
-                        getContext().getLock().lock();
+                        getContext().lock();
                         try {
                             proggress
                                     .setValue(new Float((1.0f * cent) / cents));
                             proggress.setCaption("" + 100 * cent
                                     + " rows inserted");
                         } finally {
-                            getContext().getLock().unlock();
+                            getContext().unlock();
                         }
                     }
                 } catch (SQLException e) {
@@ -95,13 +95,13 @@ public class MassInsertMemoryLeakTestApp extends LegacyApplication {
                     e.printStackTrace();
                 }
             }
-            getContext().getLock().lock();
+            getContext().lock();
             try {
                 proggress.setVisible(false);
                 proggress.setPollingInterval(0);
                 process.setEnabled(true);
             } finally {
-                getContext().getLock().unlock();
+                getContext().unlock();
             }
         }
     }