You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2018/04/19 08:32:57 UTC
[2/5] mina-sshd git commit: [SSHD-817] A few minor code fixes to
Netty code
[SSHD-817] A few minor code fixes to Netty code
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/34f1e13f
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/34f1e13f
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/34f1e13f
Branch: refs/heads/master
Commit: 34f1e13f02d1fa2c22767a08c9588c9fbfaad5dd
Parents: 229d6f3
Author: Lyor Goldstein <ly...@gmail.com>
Authored: Thu Apr 19 11:01:55 2018 +0300
Committer: Lyor Goldstein <ly...@gmail.com>
Committed: Thu Apr 19 11:36:19 2018 +0300
----------------------------------------------------------------------
.../io/DefaultIoServiceFactoryFactory.java | 30 ++++++++++----
.../org/apache/sshd/netty/NettyIoAcceptor.java | 31 +++++++-------
.../org/apache/sshd/netty/NettyIoConnector.java | 30 +++++++-------
.../sshd/netty/NettyIoServiceFactory.java | 6 ++-
.../org/apache/sshd/netty/NettyIoSession.java | 43 +++++++++++++-------
.../org/apache/sshd/netty/NettySupport.java | 7 +++-
6 files changed, 90 insertions(+), 57 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java b/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
index cfba0f4..3f4d2a4 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
@@ -46,7 +46,8 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
@Override
public IoServiceFactory create(FactoryManager manager) {
- return getFactory().create(manager);
+ IoServiceFactoryFactory factoryInstance = getFactory();
+ return factoryInstance.create(manager);
}
private IoServiceFactoryFactory getFactory() {
@@ -69,15 +70,18 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
return newInstance(clazz, factory);
}
- ClassLoader cl = Thread.currentThread().getContextClassLoader();
+ Thread currentThread = Thread.currentThread();
+ ClassLoader cl = currentThread.getContextClassLoader();
if (cl != null) {
T t = tryLoad(ServiceLoader.load(clazz, cl));
if (t != null) {
return t;
}
}
- if (cl != DefaultIoServiceFactoryFactory.class.getClassLoader()) {
- T t = tryLoad(ServiceLoader.load(clazz, DefaultIoServiceFactoryFactory.class.getClassLoader()));
+
+ ClassLoader clDefault = DefaultIoServiceFactoryFactory.class.getClassLoader();
+ if (cl != clDefault) {
+ T t = tryLoad(ServiceLoader.load(clazz, clDefault));
if (t != null) {
return t;
}
@@ -104,20 +108,28 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
public static <T extends IoServiceFactoryFactory> T newInstance(Class<T> clazz, String factory) {
BuiltinIoServiceFactoryFactories builtin = BuiltinIoServiceFactoryFactories.fromFactoryName(factory);
if (builtin != null) {
- return clazz.cast(builtin.create());
+ IoServiceFactoryFactory builtinInstance = builtin.create();
+ return clazz.cast(builtinInstance);
}
- ClassLoader cl = Thread.currentThread().getContextClassLoader();
+ Thread currentThread = Thread.currentThread();
+ ClassLoader cl = currentThread.getContextClassLoader();
if (cl != null) {
try {
- return clazz.cast(cl.loadClass(factory).newInstance());
+ Class<?> loaded = cl.loadClass(factory);
+ Object factoryInstance = loaded.newInstance();
+ return clazz.cast(factoryInstance);
} catch (Throwable t) {
LOGGER.trace("Exception while loading factory " + factory, t);
}
}
- if (cl != DefaultIoServiceFactoryFactory.class.getClassLoader()) {
+
+ ClassLoader clDefault = DefaultIoServiceFactoryFactory.class.getClassLoader();
+ if (cl != clDefault) {
try {
- return clazz.cast(DefaultIoServiceFactoryFactory.class.getClassLoader().loadClass(factory).newInstance());
+ Class<?> loaded = clDefault.loadClass(factory);
+ Object factoryInstance = loaded.newInstance();
+ return clazz.cast(factoryInstance);
} catch (Throwable t) {
LOGGER.trace("Exception while loading factory " + factory, t);
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoAcceptor.java
----------------------------------------------------------------------
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoAcceptor.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoAcceptor.java
index 8d88cdf..7f710e1 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoAcceptor.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoAcceptor.java
@@ -24,10 +24,10 @@ import java.io.InterruptedIOException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Collection;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import org.apache.sshd.common.future.CloseFuture;
@@ -55,10 +55,9 @@ import io.netty.util.concurrent.GlobalEventExecutor;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class NettyIoAcceptor extends NettyIoService implements IoAcceptor {
-
protected final ServerBootstrap bootstrap = new ServerBootstrap();
protected final DefaultCloseFuture closeFuture = new DefaultCloseFuture(toString(), lock);
- protected final Map<SocketAddress, Channel> boundAddresses = new HashMap<>();
+ protected final Map<SocketAddress, Channel> boundAddresses = new ConcurrentHashMap<>();
protected final IoHandler handler;
public NettyIoAcceptor(NettyIoServiceFactory factory, IoHandler handler) {
@@ -66,16 +65,18 @@ public class NettyIoAcceptor extends NettyIoService implements IoAcceptor {
this.handler = handler;
channelGroup = new DefaultChannelGroup("sshd-acceptor-channels", GlobalEventExecutor.INSTANCE);
bootstrap.group(factory.eventLoopGroup)
- .channel(NioServerSocketChannel.class)
- .option(ChannelOption.SO_BACKLOG, 100)
- .handler(new LoggingHandler(LogLevel.INFO))
- .childHandler(new ChannelInitializer<SocketChannel>() {
- @Override
- public void initChannel(SocketChannel ch) throws Exception {
- ChannelPipeline p = ch.pipeline();
- p.addLast(new NettyIoSession(NettyIoAcceptor.this, handler).adapter);
- }
- });
+ .channel(NioServerSocketChannel.class)
+ .option(ChannelOption.SO_BACKLOG, 100) // TODO make this configurable
+ .handler(new LoggingHandler(LogLevel.INFO)) // TODO make this configurable
+ .childHandler(new ChannelInitializer<SocketChannel>() {
+ @Override
+ public void initChannel(SocketChannel ch) throws Exception {
+ ChannelPipeline p = ch.pipeline();
+ @SuppressWarnings("resource")
+ NettyIoSession nettyIoSession = new NettyIoSession(NettyIoAcceptor.this, handler);
+ p.addLast(nettyIoSession.adapter);
+ }
+ });
}
@Override
@@ -107,7 +108,7 @@ public class NettyIoAcceptor extends NettyIoService implements IoAcceptor {
public void unbind(Collection<? extends SocketAddress> addresses) {
CountDownLatch latch = new CountDownLatch(addresses.size());
for (SocketAddress address : addresses) {
- Channel channel = boundAddresses.get(address);
+ Channel channel = boundAddresses.remove(address);
if (channel != null) {
ChannelFuture fut;
if (channel.isOpen()) {
@@ -129,7 +130,7 @@ public class NettyIoAcceptor extends NettyIoService implements IoAcceptor {
@Override
public void unbind(SocketAddress address) {
- Channel channel = boundAddresses.get(address);
+ Channel channel = boundAddresses.remove(address);
if (channel != null) {
ChannelFuture fut;
if (channel.isOpen()) {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoConnector.java
----------------------------------------------------------------------
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoConnector.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoConnector.java
index ebced0d..c485790 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoConnector.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoConnector.java
@@ -55,17 +55,18 @@ public class NettyIoConnector extends NettyIoService implements IoConnector {
this.handler = handler;
channelGroup = new DefaultChannelGroup("sshd-connector-channels", GlobalEventExecutor.INSTANCE);
bootstrap.group(factory.eventLoopGroup)
- .channel(NioSocketChannel.class)
- .option(ChannelOption.SO_BACKLOG, 100)
- .handler(new ChannelInitializer<SocketChannel>() {
- @Override
- protected void initChannel(SocketChannel ch) throws Exception {
- NettyIoSession session = new NettyIoSession(NettyIoConnector.this, handler);
- ChannelPipeline p = ch.pipeline();
- p.addLast(new LoggingHandler(LogLevel.INFO));
- p.addLast(session.adapter);
- }
- });
+ .channel(NioSocketChannel.class)
+ .option(ChannelOption.SO_BACKLOG, 100) // TODO make this configurable
+ .handler(new ChannelInitializer<SocketChannel>() {
+ @Override
+ protected void initChannel(SocketChannel ch) throws Exception {
+ @SuppressWarnings("resource")
+ NettyIoSession session = new NettyIoSession(NettyIoConnector.this, handler);
+ ChannelPipeline p = ch.pipeline();
+ p.addLast(new LoggingHandler(LogLevel.INFO)); // TODO make this configurable
+ p.addLast(session.adapter);
+ }
+ });
}
@Override
@@ -98,18 +99,19 @@ public class NettyIoConnector extends NettyIoService implements IoConnector {
@Override
public IoSession getSession() {
Object v = getValue();
- return v instanceof IoSession ? (IoSession) v : null;
+ return (v instanceof IoSession) ? (IoSession) v : null;
}
@Override
public Throwable getException() {
Object v = getValue();
- return v instanceof Throwable ? (Throwable) v : null;
+ return (v instanceof Throwable) ? (Throwable) v : null;
}
@Override
public boolean isConnected() {
- return getValue() instanceof IoSession;
+ Object v = getValue();
+ return v instanceof IoSession;
}
@Override
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoServiceFactory.java
----------------------------------------------------------------------
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoServiceFactory.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoServiceFactory.java
index 2bc3f97..f65989b 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoServiceFactory.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoServiceFactory.java
@@ -28,6 +28,7 @@ import org.apache.sshd.common.util.closeable.AbstractCloseable;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.util.concurrent.Future;
/**
* @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
@@ -43,7 +44,7 @@ public class NettyIoServiceFactory extends AbstractCloseable implements IoServic
}
public NettyIoServiceFactory(EventLoopGroup group) {
- this.eventLoopGroup = group != null ? group : new NioEventLoopGroup();
+ this.eventLoopGroup = (group != null) ? group : new NioEventLoopGroup();
this.closeEventLoopGroup = group == null;
}
@@ -60,7 +61,8 @@ public class NettyIoServiceFactory extends AbstractCloseable implements IoServic
@Override
protected CloseFuture doCloseGracefully() {
if (closeEventLoopGroup) {
- eventLoopGroup.shutdownGracefully().addListener(fut -> closeFuture.setClosed());
+ Future<?> shutdownFuture = eventLoopGroup.shutdownGracefully();
+ shutdownFuture.addListener(fut -> closeFuture.setClosed());
} else {
closeFuture.setClosed();
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
----------------------------------------------------------------------
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
index 57aecc6..f5864df 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
@@ -36,11 +36,13 @@ import org.apache.sshd.common.util.closeable.AbstractCloseable;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
+import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelPromise;
+import io.netty.util.Attribute;
/**
* The Netty based IoSession implementation.
@@ -73,22 +75,30 @@ public class NettyIoSession extends AbstractCloseable implements IoSession {
@Override
public Object getAttribute(Object key) {
- return attributes.get(key);
+ synchronized (attributes) {
+ return attributes.get(key);
+ }
}
@Override
public Object setAttribute(Object key, Object value) {
- return attributes.put(key, value);
+ synchronized (attributes) {
+ return attributes.put(key, value);
+ }
}
@Override
public Object setAttributeIfAbsent(Object key, Object value) {
- return attributes.putIfAbsent(key, value);
+ synchronized (attributes) {
+ return attributes.putIfAbsent(key, value);
+ }
}
@Override
public Object removeAttribute(Object key) {
- return attributes.remove(key);
+ synchronized (attributes) {
+ return attributes.remove(key);
+ }
}
@Override
@@ -98,13 +108,15 @@ public class NettyIoSession extends AbstractCloseable implements IoSession {
@Override
public SocketAddress getLocalAddress() {
- return context.channel().localAddress();
+ Channel channel = context.channel();
+ return channel.localAddress();
}
@Override
public IoWriteFuture writePacket(Buffer buffer) {
- ByteBuf buf = Unpooled.buffer(buffer.available());
- buf.writeBytes(buffer.array(), buffer.rpos(), buffer.available());
+ int bufLen = buffer.available();
+ ByteBuf buf = Unpooled.buffer(bufLen);
+ buf.writeBytes(buffer.array(), buffer.rpos(), bufLen);
DefaultIoWriteFuture msg = new DefaultIoWriteFuture(getRemoteAddress(), null);
ChannelPromise next = context.newPromise();
prev.addListener(whatever -> {
@@ -130,11 +142,9 @@ public class NettyIoSession extends AbstractCloseable implements IoSession {
@Override
protected CloseFuture doCloseGracefully() {
- context.writeAndFlush(Unpooled.EMPTY_BUFFER).
- addListener(ChannelFutureListener.CLOSE).
- addListener(fut -> {
- closeFuture.setClosed();
- });
+ context.writeAndFlush(Unpooled.EMPTY_BUFFER)
+ .addListener(ChannelFutureListener.CLOSE)
+ .addListener(fut -> closeFuture.setClosed());
return closeFuture;
}
@@ -146,12 +156,15 @@ public class NettyIoSession extends AbstractCloseable implements IoSession {
protected void channelActive(ChannelHandlerContext ctx) throws Exception {
context = ctx;
- service.channelGroup.add(ctx.channel());
+ Channel channel = ctx.channel();
+ service.channelGroup.add(channel);
service.sessions.put(id, NettyIoSession.this);
prev = context.newPromise().setSuccess();
- remoteAddr = context.channel().remoteAddress();
+ remoteAddr = channel.remoteAddress();
handler.sessionCreated(NettyIoSession.this);
- IoConnectFuture future = ctx.channel().attr(NettyIoService.CONNECT_FUTURE_KEY).get();
+
+ Attribute<IoConnectFuture> connectFuture = channel.attr(NettyIoService.CONNECT_FUTURE_KEY);
+ IoConnectFuture future = connectFuture.get();
if (future != null) {
future.setSession(NettyIoSession.this);
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/34f1e13f/sshd-netty/src/main/java/org/apache/sshd/netty/NettySupport.java
----------------------------------------------------------------------
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettySupport.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettySupport.java
index 9f9de89..3f4d746 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettySupport.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettySupport.java
@@ -22,13 +22,17 @@ import org.apache.sshd.common.util.Readable;
import io.netty.buffer.ByteBuf;
+/**
+ * @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
public final class NettySupport {
private NettySupport() {
throw new UnsupportedOperationException("No instance allowed");
}
- public static Readable asReadable(final ByteBuf buffer) {
+ public static Readable asReadable(ByteBuf buffer) {
return new Readable() {
@Override
public int available() {
@@ -41,5 +45,4 @@ public final class NettySupport {
}
};
}
-
}