OF-881: Simplify connection close semantics

Avoid recursion and synchronization issues

src/java/org/jivesoftware/openfire/Connection.java

@@ -144,29 +144,9 @@ public interface Connection {
      *      <li>Call notifyEvent all listeners that the channel is shutting down.
      *      <li>Close the socket.
      * </ul>
-     *
-     * An invocation of this method is equal to invoking {@link #close(boolean)} with a parameter
-     * that is false.
      */
     public void close();
 
-    /**
-     * Close this session including associated socket connection. The order of
-     * events for closing the session is:
-     * <ul>
-     *      <li>Set closing flag to prevent redundant shutdowns.
-     *      <li>Call notifyEvent all listeners that the channel is shutting down.
-     *      <li>Close the socket.
-     * </ul>
-     *
-     * This method takes into account the connection state of the peer. Specifically,
-     * when the peer is known to be in a disconnected state, no data will be sent
-     * (otherwise, this method can trigger the delivery of an end-of-stream signal).
-     *
-     * @param peerIsKnownToBeDisconnected should be set to true when the peer is known to no longer be available.
-     */
-    public void close( boolean peerIsKnownToBeDisconnected );
-
     /**
      * Notification message indicating that the server is being shutdown. Implementors
      * should send a stream error whose condition is system-shutdown before closing

src/java/org/jivesoftware/openfire/net/SocketConnection.java

@@ -468,55 +468,40 @@ public class SocketConnection implements Connection {
 
     @Override
     public void close() {
-        close( false );
-    }
-
-    @Override
-    public void close( boolean peerIsKnownToBeDisconnected ) {
-        boolean wasClosed = false;
+    	
         synchronized (this) {
-            if (!isClosed()) {
-                try {
-                    if (session != null) {
-                        session.setStatus(Session.STATUS_CLOSED);
-                    }
-
-                    if ( !peerIsKnownToBeDisconnected ) {
-                        boolean allowedToWrite = false;
-                        try {
-                            requestWriting();
-                            allowedToWrite = true;
-                            // Register that we started sending data on the connection
-                            writeStarted();
-                            writer.write("</stream:stream>");
-                            if (flashClient) {
-                                writer.write('\0');
-                            }
-                            writer.flush();
-                        }
-                        catch (IOException e) {
-                            // Do nothing
-                        }
-                        finally {
-                            // Register that we finished sending data on the connection
-                            writeFinished();
-                            if (allowedToWrite) {
-                                releaseWriting();
-                            }
-                        }
-                    }
-                }
-                catch (Exception e) {
-                    Log.error(LocaleUtils.getLocalizedString("admin.error.close")
-                            + "\n" + this.toString(), e);
-                }
-                closeConnection();
-                wasClosed = true;
+        	if (isClosed()) {
+        		return;
+        	}
+            if (session != null) {
+                session.setStatus(Session.STATUS_CLOSED);
+            }
+        }
+        
+        boolean allowedToWrite = false;
+        try {
+            requestWriting();
+            allowedToWrite = true;
+            // Register that we started sending data on the connection
+            writeStarted();
+            writer.write("</stream:stream>");
+            if (flashClient) {
+                writer.write('\0');
             }
+            writer.flush();
+        }
+        catch (Exception e) {
+            Log.error("Failed to deliver stream close tag: " + e.getMessage());
         }
-        if (wasClosed) {
-            notifyCloseListeners();
+        
+        // Register that we finished sending data on the connection
+        writeFinished();
+        if (allowedToWrite) {
+            releaseWriting();
         }
+            
+        closeConnection();
+        notifyCloseListeners();
     }
 
     @Override

src/java/org/jivesoftware/openfire/net/VirtualConnection.java

@@ -194,31 +194,25 @@ public abstract class VirtualConnection implements Connection {
      */
     @Override
     public void close() {
-        close( false );
-    }
-
-    @Override
-    public void close(boolean peerIsKnownToBeDisconnected) {
-        boolean wasClosed = false;
+    	
         synchronized (this) {
-            if (!isClosed()) {
-                try {
-                    if (session != null) {
-                        session.setStatus(Session.STATUS_CLOSED);
-                    }
-                    closeVirtualConnection();
-                }
-                catch (Exception e) {
-                    Log.error(LocaleUtils.getLocalizedString("admin.error.close")
-                            + "\n" + this.toString(), e);
-                }
-                closed = true;
-                wasClosed = true;
+        	if (isClosed()) {
+        		return;
+        	}
+            if (session != null) {
+                session.setStatus(Session.STATUS_CLOSED);
             }
         }
-        if (wasClosed) {
-            notifyCloseListeners();
+        
+        try {
+            closeVirtualConnection();
+        } catch (Exception e) {
+            Log.error(LocaleUtils.getLocalizedString("admin.error.close")
+                    + "\n" + this.toString(), e);
         }
+        
+        closed = true;
+        notifyCloseListeners();
     }
 
     @Override

src/java/org/jivesoftware/openfire/nio/NIOConnection.java

@@ -228,60 +228,33 @@ public class NIOConnection implements Connection {
 
     @Override
     public void close() {
-        close( false );
-    }
-
-    @Override
-    public void close( boolean peerIsKnownToBeDisconnected )
-    {
-        boolean notifyClose = false;
-        synchronized ( this ) {
-            try
-            {
-                if ( state == State.CLOSED )
-                {
-                    return;
-                }
-
-                // This prevents any action after the first invocation of close() on this connection.
-                if ( state != State.CLOSING )
-                {
-                    state = State.CLOSING;
-                    if ( !peerIsKnownToBeDisconnected )
-                    {
-                        try
-                        {
-                            deliverRawText( flashClient ? "</flash:stream>" : "</stream:stream>" );
-                        }
-                        catch ( Exception e )
-                        {
-                            // Ignore
-                        }
-                    }
-                }
+    	synchronized ( this ) {
+        	// prevent recursion while closing
+        	if ( state == State.CLOSED || state == State.CLOSING) {
+        		return;
+        	}
+            state = State.CLOSING;
+        }
 
-                // deliverRawText might already have forced the state from Closing to Closed. In that case, there's no need
-                // to invoke the CloseListeners again.
-                if ( state == State.CLOSING )
-                {
-                    notifyClose = true;
-                }
-            }
-            finally
-            {
-                // Ensure that the state of this connection, its session and the MINA context are eventually closed.
-                state = State.CLOSED;
-                if ( session != null )
-                {
-                    session.setStatus( Session.STATUS_CLOSED );
-                }
-                ioSession.close( true );
-            }
+        try {
+            deliverRawText( flashClient ? "</flash:stream>" : "</stream:stream>" );
+        } catch ( Exception e ) {
+            Log.error("Failed to deliver stream close tag: " + e.getMessage());
+        }
+        
+        // Ensure that the state of this connection, its session and the MINA context are eventually closed.
+        if ( session != null ) {
+            session.setStatus( Session.STATUS_CLOSED );
         }
-        if (notifyClose)
-        {
-            notifyCloseListeners(); // clean up session, etc.
+        
+        try {
+        	ioSession.close( true );
+        } catch (Exception e) {
+            Log.error("Exception while closing MINA session", e);
         }
+        
+        state = State.CLOSED;
+        notifyCloseListeners(); // clean up session, etc.
     }
 
     @Override