On Nov 15, 2007, at 3:35 PM, Paul Smith wrote:
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
I'm sure the rationale behind the original design of
SocketHubAppender is probably not in the archives, so it is hard to
tell if there was some significant motivation for having the
ServerSocket constructor on a distinct thread or if it was just
simpler to code. However, it seems undesirable to change the
behavior of SocketHubAppender when there is no known benefit to a
user of SocketHubAppender and some risk, likely slight, of losing
that desired behavior.
It doesn't seem that difficult to adapt to the changes in
ZeroConfSocketHubAppender. Since ZeroConfSocketHubAppender has never
been formally released and SocketHubAppender has been in log4j for a
long time, I'm much more inclined to make major or moderate changes
in ZeroConfSocketHubAppender than making even minor changes in
SocketHubApppender.
It shouldn't need a tricky wait mechanism, you just register in
your override of createServerSocket(). Of course, if you want
ZeroConfSocketHubAppender to work with earlier log4j's, then you'd
need to sniff either the log4j version of the presence of
SocketHubAppender.createSocketServer, but that would be a lot less
code than the log4j 1.3 sniffing already in the code.
I've taken a pass at cleaning up the old log4j 1.3 reflection code
that was calling the internal Logger.getLogger() call and replaced it
with equivalent LogLog.debug or LogLog.error code. Haven't tested
it, but I don't see any reason it wouldn't work (or couldn't be made
to work) without changing the behavior of SocketHubAppender.
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version
2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.log4j.net;
import org.apache.log4j.helpers.LogLog;
import javax.jmdns.JmDNS;
import javax.jmdns.ServiceInfo;
import java.io.IOException;
import java.net.ServerSocket;
/**
* A sub-class of SocketHubAppender that broadcasts its configuration
via Zeroconf.
*
* This allows Zeroconf aware applications such as Chainsaw to be able
to detect them, and automatically configure
* themselves to be able to connect to them.
*
* @author psmith
*
*/
public class ZeroConfSocketHubAppender extends SocketHubAppender {
public static final String
DEFAULT_ZEROCONF_ZONE="_log4j._tcp.local.";
private String zeroConfZone = DEFAULT_ZEROCONF_ZONE;
/**
* True if running on log4j 1.2.16 or later.
*/
private boolean hasCreateServerSocket;
private ServiceInfo serviceInfo;
public ZeroConfSocketHubAppender() {
setName("SocketHubAppender");
try {
SocketHubAppender.class.getMethod("createServerSocket",
new Class[] { int.class});
hasCreateServerSocket = true;
} catch(NoSuchMethodException e) {
hasCreateServerSocket = false;
}
}
public void activateOptions() {
super.activateOptions();
if (!hasCreateServerSocket) {
if (getPort() == 0) {
LogLog.warn("Unable to register
ZeroConfSocketHubAppender with unspecified port using log4j versions
prior to log4j 1.2.16");
} else {
registerService(getPort());
}
}
}
protected ServerSocket createServerSocket(final int port) throws
IOException {
ServerSocket socket = new ServerSocket(port);
registerService(socket.getLocalPort());
return socket;
}
private void registerService(int port) {
try {
JmDNS jmDNS = Zeroconf4log4j.getInstance();
serviceInfo = new ServiceInfo(zeroConfZone, getName(),
port, "SocketHubAppender on port " + port);
LogLog.debug("Registering this SocketHubAppender as :" +
serviceInfo);
jmDNS.registerService(serviceInfo);
} catch (IOException e) {
LogLog.error("Failed to instantiate JmDNS to broadcast
via ZeroConf, will now operate in simple SocketHubAppender mode");
}
}
/**
* Returns the ZeroConf domain that will be used to register
this 'device'.
*
* @return String ZeroConf zone
*/
public String getZeroConfZone() {
return zeroConfZone;
}
/**
* Sets the ZeroConf zone to register this device under, BE
CAREFUL with this value
* as ZeroConf has some weird naming conventions, it should
start with an "_" and end in a ".",
* if you're not sure about this value might I suggest that you
leave it at the default value
* which is specified in [EMAIL PROTECTED] #DEFAULT_ZEROCONF_ZONE }.
*
* This method does NO(0, zero, pun not intended) checks on this
value.
*
* @param zeroConfZone
*/
public void setZeroConfZone(String zeroConfZone) {
// TODO work out a sane checking mechanism that verifies the
value is a correct ZeroConf zone
this.zeroConfZone = zeroConfZone;
}
public synchronized void close() {
super.close();
JmDNS jmDNS = Zeroconf4log4j.getInstance();
if (serviceInfo != null) {
try {
LogLog.debug("Deregistering this SocketHubAppender
(" + serviceInfo + ")");
jmDNS.unregisterService(serviceInfo);
} catch (Exception e) {
LogLog.error("Failed to instantiate JmDNS to
broadcast via ZeroConf, will now operate in simple SocketHubAppender
mode");
}
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
