actually I've found a problem with the commit.
If ZeroconfSocketHubAppender class calls super.activateOptions() which
then creates the SocketServer, the actual creation of the ServerSocket
is done by another thread (see the SocketMonitor constructor), which
means the call back to the protected method to create the socket is
done via another thread.
This means that after super.activateOptions() call returns,
ZeroconfSocketHubAppender still doesn't know what the port is going to
be used (it's a timing thing).
I think the call to create the ServerSocket needs to be done in the
SocketMonitor constructor before the thread is started as I had
originally in my previous patch. Otherwise subclasses have to
implement a tricky wait mechanism to see what port is used before
fully activating themselves.
Can we please move that into the constructor?
Paul
On 16/11/2007, at 5:37 AM, Curt Arnold wrote:
On Nov 15, 2007, at 4:36 AM, Paul Smith wrote:
Ok, i'm a bit tired, so I ended up just going simple and pretty
much following Curt's idea, see below. Thoughts?
Index: src/main/java/org/apache/log4j/net/SocketHubAppender.java
===================================================================
--- src/main/java/org/apache/log4j/net/SocketHubAppender.java
(revision 592202)
+++ src/main/java/org/apache/log4j/net/SocketHubAppender.java
(working copy)
@@ -17,18 +17,18 @@
package org.apache.log4j.net;
-import java.util.Vector;
-import java.net.Socket;
-import java.net.ServerSocket;
-import java.net.SocketException;
-import java.io.ObjectOutputStream;
import java.io.IOException;
import java.io.InterruptedIOException;
+import java.io.ObjectOutputStream;
import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.SocketException;
+import java.util.Vector;
+import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.helpers.LogLog;
import org.apache.log4j.spi.LoggingEvent;
-import org.apache.log4j.AppenderSkeleton;
/**
Sends [EMAIL PROTECTED] LoggingEvent} objects to a set of remote log servers,
That chunk seems just like the IDE at work moving things around.
@@ -271,6 +271,19 @@
}
/**
+ * Once the [EMAIL PROTECTED] ServerSocket} is created and bound, this
method is called to notify the class the details
+ * of the address and port chosen. In the standard
SocketHubAppender case this will be the
+ * requested port, but if the port is specified as 0, then a
random port is chosen by the underlying OS.
+ * Child classes may be interested in the actual port chosen,
and this method may be overridden
+ * and used to gain the details of the actual socket being
listened on.
+ * @param address
+ * @param actualPortUsed
+ */
+ protected void socketBound(InetAddress address, int
actualPortUsed) {
+
+ }
+
+ /**
This class is used internally to monitor a ServerSocket
and register new connections in a vector passed in the
constructor. */
Will discuss later in message.
@@ -280,6 +293,7 @@
private Vector oosList;
private boolean keepRunning;
private Thread monitorThread;
+ private ServerSocket serverSocket;
/**
Create a thread and start the monitor. */
@@ -288,9 +302,15 @@
port = _port;
oosList = _oosList;
keepRunning = true;
+ try {
+ serverSocket = new ServerSocket(port);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to create a
ServerSocket", e);
+ }
monitorThread = new Thread(this);
monitorThread.setDaemon(true);
monitorThread.start();
+ socketBound(serverSocket.getInetAddress(),
serverSocket.getLocalPort());
}
/**
@@ -320,9 +340,8 @@
they connect to the socket. */
public
void run() {
- ServerSocket serverSocket = null;
+
try {
- serverSocket = new ServerSocket(port);
serverSocket.setSoTimeout(1000);
}
catch (Exception e) {
This still moves behavior around a bit. I noticed that the inner
class is not defined as static, so it has access to all the methods
and members of its enclosing class, so you could keep the same
notification method, but minimize the changes to the SocketMonitor
by just calling socketBound after the existing socket creation code.
@@ -323,6 +336,7 @@
ServerSocket serverSocket = null;
try {
serverSocket = new ServerSocket(port);
+ socketBound(serverSocket.getInetAddress(),
serverSocket.getLocalPort());
serverSocket.setSoTimeout(1000);
}
catch (Exception e) {
But I didn't see much benefit from hiding the rest of the
serverSocket from the extending class, so I first simplified the
code to:
@@ -323,6 +336,7 @@
ServerSocket serverSocket = null;
try {
serverSocket = new ServerSocket(port);
+ socketBound(serverSocket);
serverSocket.setSoTimeout(1000);
}
catch (Exception e) {
which lets the extending class extract as much or little info out of
the class as possible. But once I did that, it seemed even more
simple to use a protected method to create the socket which is
easier to explain and lets the extending class do everything the
other approaches do and more.
Index: src/main/java/org/apache/log4j/net/SocketHubAppender.java
===================================================================
--- src/main/java/org/apache/log4j/net/SocketHubAppender.java
(revision 592094)
+++ src/main/java/org/apache/log4j/net/SocketHubAppender.java
(working copy)
@@ -271,6 +271,16 @@
}
/**
+ * Creates a server socket to accept connections.
+ * @param socketPort port on which the socket should listen, may
be zero.
+ * @return new socket.
+ * @throws IOException IO error when opening the socket.
+ */
+ protected ServerSocket createServerSocket(final int socketPort)
throws IOException {
+ return new ServerSocket(socketPort);
+ }
+
+ /**
This class is used internally to monitor a ServerSocket
and register new connections in a vector passed in the
constructor. */
@@ -322,7 +332,7 @@
void run() {
ServerSocket serverSocket = null;
try {
- serverSocket = new ServerSocket(port);
+ serverSocket = createServerSocket(port);
serverSocket.setSoTimeout(1000);
}
catch (Exception e) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Paul Smith
Core Engineering Manager
Aconex
The easy way to save time and money on your project
696 Bourke Street, Melbourne,
VIC 3000, Australia
Tel: +61 3 9240 0200 Fax: +61 3 9240 0299
Email: [EMAIL PROTECTED] www.aconex.com
This email and any attachments are intended solely for the addressee.
The contents may be privileged, confidential and/or subject to
copyright or other applicable law. No confidentiality or privilege is
lost by an erroneous transmission. If you have received this e-mail in
error, please let us know by reply e-mail and delete or destroy this
mail and all copies. If you are not the intended recipient of this
message you must not disseminate, copy or take any action in reliance
on it. The sender takes no responsibility for the effect of this
message upon the recipient's computer system.
