ftp: Fix deadlock introduced in recent refactoring

Recent changes to the FTP door introduced a deadlock:

"pool-116-thread-7":
        at org.dcache.ftp.door.AbstractFtpDoorV1.setTransfer(AbstractFtpDoorV1.java:2380)
        - waiting to lock <0x00000007c5eade98> (a org.dcache.ftp.door.GsiFtpDoorV1)
        at org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer.transferCompleted(AbstractFtpDoorV1.java:1022)
        - locked <0x00000007c5eb3860> (a org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer)
        at org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer.finished(AbstractFtpDoorV1.java:972)
        at org.dcache.util.Transfer.finished(Transfer.java:464)
        - locked <0x00000007c5eb3860> (a org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer)
        at org.dcache.ftp.door.AbstractFtpDoorV1.messageArrived(AbstractFtpDoorV1.java:1444)
        at sun.reflect.GeneratedMethodAccessor132.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:606)
        at org.dcache.cells.CellMessageDispatcher$ShortReceiver.deliver(CellMessageDispatcher.java:289)
        at org.dcache.cells.CellMessageDispatcher.call(CellMessageDispatcher.java:202)
        at org.dcache.cells.AbstractCell.messageArrived(AbstractCell.java:859)
        at dmg.cells.nucleus.CellAdapter.messageArrived(CellAdapter.java:981)
        at dmg.cells.nucleus.CellNucleus$DeliverMessageTask.innerRun(CellNucleus.java:1037)
        at dmg.cells.nucleus.CellNucleus$AbstractNucleusTask.run(CellNucleus.java:946)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
        at java.lang.Thread.run(Thread.java:744)
"Thread-160":
        at org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer.abort(AbstractFtpDoorV1.java:1060)
        - waiting to lock <0x00000007c5eb3860> (a org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer)
        at org.dcache.ftp.door.AbstractFtpDoorV1$FtpTransfer.abort(AbstractFtpDoorV1.java:1036)
        at org.dcache.ftp.door.AbstractFtpDoorV1.shutdown(AbstractFtpDoorV1.java:1363)
        - locked <0x00000007c5eade98> (a org.dcache.ftp.door.GsiFtpDoorV1)
        at diskCacheV111.doors.LineBasedDoor$CommandQueue.stop(LineBasedDoor.java:343)
        - locked <0x00000007c5ed1290> (a diskCacheV111.doors.LineBasedDoor$CommandQueue)
        at diskCacheV111.doors.LineBasedDoor.run(LineBasedDoor.java:176)
        at java.lang.Thread.run(Thread.java:744)

This deadlock is caused by unnecessary synchronization in the shutdown
method. This synchronization did not exist in the old code.

The patch also adds an accessor for the transfer field, thus solving
potential race conditions.

Target: trunk
Require-notes: no
Require-book: no
Acked-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Patch: http://rb.dcache.org/r/6318/

modules/dcache-ftp/src/main/java/org/dcache/ftp/door/AbstractFtpDoorV1.java

@@ -1315,7 +1315,7 @@ public Object ac_get_door_info(Args args)
         long[] uids = (_subject != null) ? Subjects.getUids(_subject) : new long[0];
         doorInfo.setOwner((uids.length == 0) ? "0" : Long.toString(uids[0]));
         doorInfo.setProcess("0");
-        FtpTransfer transfer = _transfer;
+        FtpTransfer transfer = getTransfer();
         if (transfer != null) {
             IoDoorEntry[] entries = { transfer.getIoDoorEntry() };
             doorInfo.setIoDoorEntries(entries);
@@ -1365,14 +1365,12 @@ public void ftpcommand(String cmdline)
 
         // If a transfer is in progress, only permit ABORT and a few
         // other commands to be processed
-        synchronized(this) {
-            if (_transfer != null &&
+        if (getTransfer() != null &&
                 !(cmd.equals("abor") || cmd.equals("mic")
-                  || cmd.equals("conf") || cmd.equals("enc")
-                  || cmd.equals("quit") || cmd.equals("bye"))) {
-                reply("503 Transfer in progress", false);
-                return;
-            }
+                        || cmd.equals("conf") || cmd.equals("enc")
+                        || cmd.equals("quit") || cmd.equals("bye"))) {
+            reply("503 Transfer in progress", false);
+            return;
         }
 
         if (!_methodDict.containsKey(cmd)) {
@@ -1426,13 +1424,13 @@ private synchronized void closePassiveModeServerSocket()
     }
 
     @Override
-    public synchronized void shutdown()
+    public void shutdown()
     {
         super.shutdown();
 
         /* In case of failure, we may have a transfer hanging around.
          */
-        FtpTransfer transfer = _transfer;
+        FtpTransfer transfer = getTransfer();
         if (transfer != null) {
             transfer.abort(451, "Aborting transfer due to session termination");
         }
@@ -1503,7 +1501,7 @@ public void messageArrived(CellMessage envelope,
                                 GFtpTransferStartedMessage message)
      {
          LOGGER.debug("Received TransferStarted message");
-         FtpTransfer transfer = _transfer;
+         FtpTransfer transfer = getTransfer();
          if (transfer != null) {
              transfer.transferStarted(envelope, message);
          }
@@ -1513,7 +1511,7 @@ public void messageArrived(DoorTransferFinishedMessage message)
     {
         LOGGER.debug("Received TransferFinished message [rc={}]",
                 message.getReturnCode());
-        FtpTransfer transfer = _transfer;
+        FtpTransfer transfer = getTransfer();
         if (transfer != null) {
             transfer.finished(message);
         }
@@ -2543,11 +2541,15 @@ public void ftp_retr(String arg)
         }
     }
 
-    protected synchronized Transfer setTransfer(FtpTransfer transfer)
+    protected synchronized FtpTransfer getTransfer()
+    {
+        return _transfer;
+    }
+
+    protected synchronized void setTransfer(FtpTransfer transfer)
     {
         _transfer = transfer;
         notifyAll();
-        return transfer;
     }
 
     protected synchronized void joinTransfer()
@@ -3233,7 +3235,7 @@ public void ftp_abor(String arg)
     {
         checkLoggedIn();
 
-        FtpTransfer transfer = _transfer;
+        FtpTransfer transfer = getTransfer();
         if (transfer != null) {
             transfer.abort(426, "Transfer aborted");
         }