Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ThreadDeathWatcher causes custom classLoader script memory leaks #7290

Closed
juebanlin opened this issue Oct 10, 2017 · 7 comments
Closed

ThreadDeathWatcher causes custom classLoader script memory leaks #7290

juebanlin opened this issue Oct 10, 2017 · 7 comments
Assignees
Labels
Milestone

Comments

@juebanlin
Copy link

@juebanlin juebanlin commented Oct 10, 2017

Expected behavior

Actual behavior

Steps to reproduce

`package net.jueb.redAlert.script.game.serverScript.sys;

import io.netty.buffer.ByteBufAllocator;
import net.jueb.redAlert.core.base.message.AppByteMessage;
import net.jueb.redAlert.core.base.message.MsgCode;
import net.jueb.redAlert.game.ServerConfig;
import net.jueb.redAlert.game.ServerMain;
import net.jueb.redAlert.game.factory.ScriptFactory;
import net.jueb.util4j.hotSwap.annationScriptFactory.annations.AnnationScript;

/**

  • 调试脚本

  • @author Administrator
    */
    @AnnationScript(id = MsgCode.Game_Sys_Debug)
    public class DebugScript extends AbstractSysScript{

    @override
    public void action() {
    ClassLoader customLoader=ServerConfig.classProvider.getClassLoader();
    if(getClass().getClassLoader()==customLoader)
    {//this is hotswap script
    //test1
    ByteBufAllocator.DEFAULT.ioBuffer();// init ThreadDeathWatcher watcher thread
    //or test2 ,use netty client write msg to init ThreadDeathWatcher
    ServerMain.getInstance().getLogClient().sendMessage(new AppByteMessage(AppByteMessage.Heart_Req));
    //so ,this script started netty thread
    }
    }
    public static void main(String[] args) {
    //run DebugScript action method
    ScriptFactory.getInstance().buildAction(MsgCode.Game_Sys_Debug).run();
    }
    }`

Minimal yet complete reproducer code (or URL to code)

default

Netty version

4.1.6

JVM version (e.g. java -version)

OS version (e.g. uname -a)

@juebanlin

This comment has been minimized.

Copy link
Author

@juebanlin juebanlin commented Oct 10, 2017

I hope you can initialize manually

Or pre initialization ThreadDeathWatcher

@normanmaurer

This comment has been minimized.

Copy link
Member

@normanmaurer normanmaurer commented Oct 22, 2017

@juebanlin sorry but I not understand what exactly is the problem and how to reproduce. Can you elaborate ?

@juebanlin

This comment has been minimized.

Copy link
Author

@juebanlin juebanlin commented Oct 25, 2017

@normanmaurer

There is a custom classloader loaded script,

If script creates a PooledByteBuf, it may trigger the creation of a thread,

Reference ThreadDeathWatcher's schedule method,

And then the script of this classloader is not recyclable,

Because the netty thread does not change before the JVM closes, the hot update system creates new classloader at any time,

Finally, memory leaks occur in the thermal update system.

@normanmaurer

This comment has been minimized.

Copy link
Member

@normanmaurer normanmaurer commented Oct 26, 2017

@juebanlin I still not understand it completely... Can you provide a reproducer ?

@juebanlin

This comment has been minimized.

Copy link
Author

@juebanlin juebanlin commented Oct 26, 2017

@normanmaurer
I'm not good at English.I paste the code

Maven Dependencies

<dependency>
    <groupId>net.jueb</groupId>
    <artifactId>util4j</artifactId>
    <version>4.1.3</version>
</dependency>

demo code:

public class Demo {
	public interface Code{
		public static final int Event_OnConnection=1001;
		public static final int Msg_Login=1002;
	}
	public interface Script extends Runnable{
		
		public int code();
		public void run();
		public void setParams(Object ...params);
		public Object[] getParams();
	}
	public abstract class AbstractScript implements Script{
		Object[] params;
		@Override
		public void setParams(Object... params) {
			this.params=params;
		}
		@Override
		public Object[] getParams() {
			return params;
		}
	}
	public class HotSwapScriptPrivder extends MapClassProvider<Script,Integer>{
		protected HotSwapScriptPrivder(IClassProvider classProvider) {
			super(Script.class, classProvider);
		}
		@Override
		protected Integer findKey(Script instance) {
			return instance.code();
		}
	}
	
	ScheduledExecutorService exec=Executors.newScheduledThreadPool(4);
	QueueGroupExecutor ge=buildByMpMc(2, 10, 1000);
	protected QueueGroupExecutor buildByMpMc(int min,int max,int maxPendingTask)
	{
		int maxQueueCount=maxPendingTask;
		//多生产多消费者队列(线程竞争队列)
		Queue<Runnable> bossQueue=new MpmcAtomicArrayQueue<>(maxQueueCount);
		QueueFactory qf=new QueueFactory() {
			@Override
			public RunnableQueue buildQueue() {
				//多生产单消费者队列(PS:bossQueue决定了一个队列只能同时被一个线程处理)
				Queue<Runnable> queue=MpscLinkedQueue.newMpscLinkedQueue();
				return new RunnableQueueWrapper(queue);
			}
		};
		IndexQueueGroupManager iqm=new DefaultIndexQueueManager(qf,false);
		KeyQueueGroupManager kqm=new DefaultKeyQueueManager(qf);
		DefaultQueueGroupExecutor.Builder b=new DefaultQueueGroupExecutor.Builder();
		b.setAssistExecutor(Executors.newSingleThreadExecutor());
		return b.setMaxPoolSize(max).setCorePoolSize(min).setBossQueue(bossQueue).setIndexQueueGroupManager(iqm).setKeyQueueGroupManagerr(kqm).build();
	}
	
	public void execute(String queueIndex,Runnable task)
	{
		ge.execute(queueIndex, task);
	}
	
	public void testStart() throws Exception
	{
		//step1 init hotswap system,it auto update class.
		String scriptLib="d:/demoLib/";
		DefaultClassSource classSource=new DefaultClassSource();
		classSource.addJarDir(new URL(scriptLib).toURI());
		classSource.updateAttach(exec, TimeUnit.SECONDS, 10);
		IClassProvider classPrivder=new DynamicClassProvider(classSource);
		classPrivder.setAutoReload(true);
		final HotSwapScriptPrivder testClassPrivder=new HotSwapScriptPrivder(classPrivder);
		//step2 start nettClient
		String demoHost="127.0.0.1";
		int demoPort=1001;
        EventLoopGroup group = new NioEventLoopGroup();
        try {
            Bootstrap b = new Bootstrap();
            b.group(group)
             .channel(NioSocketChannel.class)
             .option(ChannelOption.TCP_NODELAY, true)
             .handler(new ChannelInitializer<SocketChannel>() {
                 @Override
                 public void initChannel(SocketChannel ch) throws Exception {
                     ChannelPipeline p = ch.pipeline();
                     p.addLast(new ChannelInboundHandlerAdapter() {
                    	 @Override
                    	public void channelActive(ChannelHandlerContext ctx) throws Exception {
                    		Script script= testClassPrivder.buildInstance(Code.Event_OnConnection,ctx);
                    		execute(ctx.channel().id().asLongText(), script);//step3 handle logic
                    	}
                     });
                 }
             });
            ChannelFuture f = b.connect(demoHost, demoPort).sync();
            f.channel().closeFuture().sync();
        } finally {
            group.shutdownGracefully();
        }
	}
	
	public class ConnectionOpenScript extends AbstractScript{
		@Override
		public int code() {
			return Code.Event_OnConnection;
		}
		@Override
		public void run() {
			//step4
			ChannelHandlerContext ctx=(ChannelHandlerContext) getParams()[0];
			ByteBuf loginMsg=ctx.alloc().buffer();
			//....build Msg Content
			ctx.write(loginMsg);
			/**
			 * it may trigger the creation of a thread ==> ThreadDeathWatcher's schedule method
			 * and the watcher Thread holds classLoader reference
			 */
		}
	}
	public static void main(String[] args) throws Exception {
		Demo t=new Demo();
		t.testStart();
	}
}

current fix

If I'm going to fix this classLoader memory leak problem, I have to initialize watcherThread in advance

public void testStart() throws Exception
	{
		PooledByteBufAllocator.DEFAULT.ioBuffer();//Initialization Watcher Thread
		//step1 init hotswap system,it auto update class.
		//...
	}
@normanmaurer normanmaurer self-assigned this Dec 11, 2017
@normanmaurer normanmaurer added the defect label Dec 11, 2017
normanmaurer added a commit that referenced this issue Dec 11, 2017
…sloader leaks.

Motivation:

ThreadDeathWatcher and GlobalEventExecutor may create and start a new thread from various other threads and so inherit the classloader. We need to ensure we not inherit to allow recycling the classloader.

Modifications:

Use Thread.setContextClassLoader(null) to ensure we not hold a strong reference to the classloader and so not leak it.

Result:

Fixes [#7290].
@normanmaurer normanmaurer added this to the 4.0.55.Final milestone Dec 11, 2017
@normanmaurer

This comment has been minimized.

Copy link
Member

@normanmaurer normanmaurer commented Dec 11, 2017

@juebanlin can you check #7493 ?

normanmaurer added a commit that referenced this issue Dec 12, 2017
…sloader leaks.

Motivation:

ThreadDeathWatcher and GlobalEventExecutor may create and start a new thread from various other threads and so inherit the classloader. We need to ensure we not inherit to allow recycling the classloader.

Modifications:

Use Thread.setContextClassLoader(null) to ensure we not hold a strong reference to the classloader and so not leak it.

Result:

Fixes [#7290].
normanmaurer added a commit that referenced this issue Dec 12, 2017
…sloader leaks.

Motivation:

ThreadDeathWatcher and GlobalEventExecutor may create and start a new thread from various other threads and so inherit the classloader. We need to ensure we not inherit to allow recycling the classloader.

Modifications:

Use Thread.setContextClassLoader(null) to ensure we not hold a strong reference to the classloader and so not leak it.

Result:

Fixes [#7290].
@normanmaurer

This comment has been minimized.

Copy link
Member

@normanmaurer normanmaurer commented Dec 12, 2017

Fixed by #7493

kiril-me added a commit to kiril-me/netty that referenced this issue Feb 28, 2018
…sloader leaks.

Motivation:

ThreadDeathWatcher and GlobalEventExecutor may create and start a new thread from various other threads and so inherit the classloader. We need to ensure we not inherit to allow recycling the classloader.

Modifications:

Use Thread.setContextClassLoader(null) to ensure we not hold a strong reference to the classloader and so not leak it.

Result:

Fixes [netty#7290].
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.