Bug 44108

Here is a refactored and bug fixed version of the 2nd half of this class - the
SocketHandler part.

This corrects the bug that causes it to miss messages, cleans up some areas of
the code where the print writers were not closed properly, and makes the code
safe for multi-threaded operation - the previous code was not safe, because it
was using two vectors that were assumed to be the same size - but in reality,
they could be different sizes depending on how the threads stop and start.

You may need to remove the generics, if you still need to support 1.4.



// ---------------------------------------------------------- SocketHandler:

	/**
	 * The SocketHandler class is used to accept connections from clients. It is
	 * threaded so that clients can connect/disconnect asynchronously.
	 */
	protected class SocketHandler extends Thread
	{
		private Vector<TelnetClient> connections = new Vector<TelnetClient>();
		private ServerSocket serverSocket;
		private int MAX_CONNECTIONS = 20;

		public void finalize()
		{
			close();
		}

		/**
		 * make sure we close all network connections when this handler is
		 * destroyed.
		 * 
		 * @since 1.2.15
		 */
		public void close()
		{
			for (Enumeration<TelnetClient> e = connections.elements(); e.hasMoreElements();)
			{
				try
				{
					e.nextElement().close();
				}
				catch (Exception ex)
				{
				}
			}

			try
			{
				serverSocket.close();
			}
			catch (Exception ex)
			{
			}
		}

		/** sends a message to each of the clients in telnet-friendly output. */
		public void send(String message)
		{
			Vector<TelnetClient> lostClients = new Vector<TelnetClient>();
			for (Enumeration<TelnetClient> e = connections.elements(); e.hasMoreElements();)
			{
				TelnetClient tc = e.nextElement();
				tc.writer.print(message);
				if (tc.writer.checkError())
				{
					// The client has closed the connection, remove it from our list:
					lostClients.add(tc);
				}
			}
			
			for (Enumeration<TelnetClient> e = lostClients.elements(); e.hasMoreElements();)
			{
				TelnetClient tc = e.nextElement();
				tc.close();
				connections.remove(tc);
			}
			
		}

		/**
		 * Continually accepts client connections. Client connections are
		 * refused when MAX_CONNECTIONS is reached.
		 */
		public void run()
		{
			while (!serverSocket.isClosed())
			{
				try
				{
					Socket newClient = serverSocket.accept();
					TelnetClient tc = new TelnetClient(newClient);
					if (connections.size() < MAX_CONNECTIONS)
					{
						connections.addElement(tc);
						tc.writer.println("TelnetAppender v1.0 (" + connections.size() + " active
connections)");
						tc.writer.println();
						tc.writer.flush();
					}
					else
					{
						tc.writer.println("Too many connections.");
						tc.writer.flush();
						tc.close();
					}
				}
				catch (Exception e)
				{
					if (!serverSocket.isClosed())
					{
						LogLog.error("Encountered error while in SocketHandler loop.", e);
					}
					break;
				}
			}

			try
			{
				serverSocket.close();
			}
			catch (IOException ex)
			{
			}
		}

		public SocketHandler(int port) throws IOException
		{
			serverSocket = new ServerSocket(port);
			setName("TelnetAppender-" + getName() + "-" + port);
		}

	}

	protected class TelnetClient
	{
		private PrintWriter writer;
		private Socket connection;

		protected TelnetClient(Socket socket) throws IOException
		{
			connection = socket;
			writer = new PrintWriter(connection.getOutputStream());
		}

		protected void close()
		{
			if (writer != null)
			{
				try
				{
					writer.flush();
					writer.close();
				}
				catch (Exception e)
				{
					// noop
				}
			}

			if (connection != null)
			{
				try
				{
					connection.close();
				}
				catch (Exception e)
				{
					// noop
				}
			}
		}
	}

1 more issue regarding line feeds - one in my new code, one in the other
existing code.

My code posted above needs this replacement (I was using system line feeds, and
not the line feeds called out in the telnet RFC):

if (connections.size() < MAX_CONNECTIONS)
					{
						connections.addElement(tc);
						tc.writer.print("TelnetAppender v1.0 (" + connections.size() + " active
connections)\r\n\r\n");
						tc.writer.flush();
					}
					else
					{
						tc.writer.print("Too many connections.\r\n");
						tc.writer.flush();
						tc.close();
					}



Finally, the append method should have this in it:

for (int i = 0; i < len; i++)
{
	sh.send(s[i] + "\r\n");
}

The existing code uses the system line feed character instead of the correct
telnet line feed.

Thanks for the bug report and patches.  log4j 1.2.x targets JDK 1.2 and later, so can't use generics in its 
code.  Will likely not get attention until after Christmas, sorry.

Committed a change in rev 612959 that should address the same issues with less code changes.

 JDK 1.2 added Iterator as a replacement for Enumerator and Iterator.remove() can safely remove an 
element while iterating.  So SocketHandler.send was rewritten using Iterator. 

The occasions that writers and connections could get out of sync would be when a connection is being 
accepted while a message is being sent.  I've added sync blocks so that any access to connections or 
writers must have a lock of SocketHandler.  You could rewrite everything so that you could safely 
accept a connection while messages were being sent, but that seems to be overkill.

The Telnet RFC does require CRLF's.  The main message requires that you specify the layout including 
the line feed, so you should not use %n in a layout with TelnetAppender.  It would be possible to fix 
this after the fact, but I haven't attempted that.  I did change the EOL's used in the exception messages.

Changes look good to me.