Some more improvements to threading

- Put all ComputerCraft threads into a thread group
 - Be a little more aggressive in when we cleanup/abandon threads.

src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java

@@ -8,7 +8,7 @@ package dan200.computercraft.core.apis.http;
 
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import dan200.computercraft.shared.util.ThreadUtils;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.nio.NioEventLoopGroup;
 
@@ -25,17 +25,13 @@ public final class HTTPExecutor
         4, Integer.MAX_VALUE,
         60L, TimeUnit.SECONDS,
         new SynchronousQueue<>(),
-        new ThreadFactoryBuilder()
+        ThreadUtils.builder( "HTTP" )
-            .setDaemon( true )
             .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 )
-            .setNameFormat( "ComputerCraft-HTTP-%d" )
             .build()
     ) );
 
-    public static final EventLoopGroup LOOP_GROUP = new NioEventLoopGroup( 4, new ThreadFactoryBuilder()
+    public static final EventLoopGroup LOOP_GROUP = new NioEventLoopGroup( 4, ThreadUtils.builder( "Netty" )
-        .setDaemon( true )
         .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 )
-        .setNameFormat( "ComputerCraft-Netty-%d" )
         .build()
     );
 

src/main/java/dan200/computercraft/core/computer/ComputerThread.java

@@ -8,13 +8,14 @@ package dan200.computercraft.core.computer;
 
 import dan200.computercraft.ComputerCraft;
 import dan200.computercraft.core.tracking.Tracking;
+import dan200.computercraft.shared.util.ThreadUtils;
 
 import java.util.HashSet;
 import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.ThreadFactory;
 
 public class ComputerThread
 {
@@ -56,8 +57,8 @@ public class ComputerThread
      */
     private static Thread[] s_threads = null;
 
-    private static final AtomicInteger s_ManagerCounter = new AtomicInteger( 1 );
+    private static final ThreadFactory s_ManagerFactory = ThreadUtils.factory( "Computer-Manager" );
-    private static final AtomicInteger s_DelegateCounter = new AtomicInteger( 1 );
+    private static final ThreadFactory s_RunnerFactory = ThreadUtils.factory( "Computer-Runner" );
 
     /**
      * Start the computer thread
@@ -72,16 +73,12 @@ public class ComputerThread
                 s_threads = new Thread[ComputerCraft.computer_threads];
             }
 
-            SecurityManager manager = System.getSecurityManager();
-            final ThreadGroup group = manager == null ? Thread.currentThread().getThreadGroup() : manager.getThreadGroup();
             for( int i = 0; i < s_threads.length; i++ )
             {
                 Thread thread = s_threads[i];
                 if( thread == null || !thread.isAlive() )
                 {
-                    thread = s_threads[i] = new Thread( group, new TaskExecutor(), "ComputerCraft-Computer-Manager-" + s_ManagerCounter.getAndIncrement() );
+                    (s_threads[i] = s_ManagerFactory.newThread( new TaskExecutor() )).start();
-                    thread.setDaemon( true );
-                    thread.start();
                 }
             }
         }
@@ -188,12 +185,7 @@ public class ComputerThread
             if( thread == null || !thread.isAlive() )
             {
                 runner = new TaskRunner();
-
+                (thread = s_RunnerFactory.newThread( runner )).start();
-                SecurityManager manager = System.getSecurityManager();
-                final ThreadGroup group = manager == null ? Thread.currentThread().getThreadGroup() : manager.getThreadGroup();
-                Thread thread = this.thread = new Thread( group, runner, "ComputerCraft-Computer-Runner" + s_DelegateCounter.getAndIncrement() );
-                thread.setDaemon( true );
-                thread.start();
             }
 
             long start = System.nanoTime();

src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java

@@ -6,7 +6,6 @@
 
 package dan200.computercraft.core.lua;
 
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import dan200.computercraft.ComputerCraft;
 import dan200.computercraft.api.lua.*;
 import dan200.computercraft.core.computer.Computer;
@@ -14,6 +13,7 @@ import dan200.computercraft.core.computer.ITask;
 import dan200.computercraft.core.computer.MainThread;
 import dan200.computercraft.core.tracking.Tracking;
 import dan200.computercraft.core.tracking.TrackingField;
+import dan200.computercraft.shared.util.ThreadUtils;
 import org.squiddev.cobalt.*;
 import org.squiddev.cobalt.compiler.CompileException;
 import org.squiddev.cobalt.compiler.LoadState;
@@ -48,18 +48,15 @@ public class CobaltLuaMachine implements ILuaMachine
         0, Integer.MAX_VALUE,
         60L, TimeUnit.SECONDS,
         new SynchronousQueue<>(),
-        new ThreadFactoryBuilder()
+        ThreadUtils.factory( "Coroutine" )
-            .setDaemon( true )
-            .setNameFormat( "ComputerCraft-Coroutine-%d" )
-            .build()
     );
 
     private final Computer m_computer;
 
-    private final LuaState m_state;
+    private LuaState m_state;
-    private final LuaTable m_globals;
+    private LuaTable m_globals;
-
     private LuaThread m_mainRoutine;
+
     private String m_eventFilter;
     private String m_softAbortMessage;
     private String m_hardAbortMessage;
@@ -132,6 +129,7 @@ public class CobaltLuaMachine implements ILuaMachine
                         Tracking.addValue( m_computer, TrackingField.COROUTINES_DISPOSED, 1 );
                     }
                 } );
+                ComputerCraft.log.info( "Thread pool: " + coroutines );
             } )
             .build();
 
@@ -189,10 +187,7 @@ public class CobaltLuaMachine implements ILuaMachine
     public void loadBios( InputStream bios )
     {
         // Begin executing a file (ie, the bios)
-        if( m_mainRoutine != null )
+        if( m_mainRoutine != null ) return;
-        {
-            return;
-        }
 
         try
         {
@@ -201,30 +196,19 @@ public class CobaltLuaMachine implements ILuaMachine
         }
         catch( CompileException e )
         {
-            if( m_mainRoutine != null )
+            unload();
-            {
-                m_mainRoutine.abandon();
-                m_mainRoutine = null;
-            }
         }
         catch( IOException e )
         {
             ComputerCraft.log.warn( "Could not load bios.lua ", e );
-            if( m_mainRoutine != null )
+            unload();
-            {
-                m_mainRoutine.abandon();
-                m_mainRoutine = null;
-            }
         }
     }
 
     @Override
     public void handleEvent( String eventName, Object[] arguments )
     {
-        if( m_mainRoutine == null )
+        if( m_mainRoutine == null ) return;
-        {
-            return;
-        }
 
         if( m_eventFilter != null && eventName != null && !eventName.equals( m_eventFilter ) && !eventName.equals( "terminate" ) )
         {
@@ -251,26 +235,14 @@ public class CobaltLuaMachine implements ILuaMachine
             else
             {
                 LuaValue filter = results.arg( 2 );
-                if( filter.isString() )
+                m_eventFilter = filter.isString() ? filter.toString() : null;
-                {
-                    m_eventFilter = filter.toString();
-                }
-                else
-                {
-                    m_eventFilter = null;
-                }
             }
 
-            LuaThread mainThread = m_mainRoutine;
+            if( m_mainRoutine.getStatus().equals( "dead" ) ) unload();
-            if( mainThread.getStatus().equals( "dead" ) )
-            {
-                m_mainRoutine = null;
-            }
         }
         catch( LuaError e )
         {
-            m_mainRoutine.abandon();
+            unload();
-            m_mainRoutine = null;
         }
         finally
         {
@@ -307,18 +279,18 @@ public class CobaltLuaMachine implements ILuaMachine
     @Override
     public boolean isFinished()
     {
-        return (m_mainRoutine == null);
+        return m_mainRoutine == null;
     }
 
     @Override
     public void unload()
     {
-        if( m_mainRoutine != null )
+        if( m_state == null ) return;
-        {
+
-            LuaThread mainThread = m_mainRoutine;
+        m_state.abandon();
-            mainThread.abandon();
+        m_mainRoutine = null;
-            m_mainRoutine = null;
+        m_state = null;
-        }
+        m_globals = null;
     }
 
     private LuaTable wrapLuaObject( ILuaObject object )
@@ -714,7 +686,7 @@ public class CobaltLuaMachine implements ILuaMachine
         private byte[] bytes;
         private int offset, remaining = 0;
 
-        public StringInputStream( LuaState state, LuaValue func )
+        StringInputStream( LuaState state, LuaValue func )
         {
             this.state = state;
             this.func = func;

src/main/java/dan200/computercraft/shared/util/ThreadUtils.java

@@ -0,0 +1,68 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2018. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+
+package dan200.computercraft.shared.util;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+
+import java.util.concurrent.ThreadFactory;
+
+/**
+ * Provides some utilities to create thread groups
+ */
+public final class ThreadUtils
+{
+    private static final ThreadGroup baseGroup = new ThreadGroup( "ComputerCraft" );
+
+    private ThreadUtils()
+    {
+    }
+
+    /**
+     * Construct a group under ComputerCraft's shared group
+     *
+     * @param name The group's name. This will be prefixed with "ComputerCraft-".
+     * @return The constructed thread group.
+     */
+    public static ThreadGroup group( String name )
+    {
+        return new ThreadGroup( baseGroup, baseGroup.getName() + "-" + name );
+    }
+
+    /**
+     * Create a new {@link ThreadFactoryBuilder}, which constructs threads under a group of the given {@code name}.
+     *
+     * Each thread will be of the format {@code ComputerCraft-<name>-<number>}, and belong to a group
+     * called {@code ComputerCraft-<name>} (which in turn will be a child group of the main {@code ComputerCraft} group.
+     *
+     * @param name The name for the thread group and child threads.
+     * @return The constructed thread factory builder, which may be extended with other properties.
+     * @see #factory(String)
+     */
+    public static ThreadFactoryBuilder builder( String name )
+    {
+        ThreadGroup group = group( name );
+        return new ThreadFactoryBuilder()
+            .setDaemon( true )
+            .setNameFormat( group.getName().replace( "%", "%%" ) + "-%d" )
+            .setThreadFactory( x -> new Thread( group, x ) );
+    }
+
+    /**
+     * Create a new {@link ThreadFactory}, which constructs threads under a group of the given {@code name}.
+     *
+     * Each thread will be of the format {@code ComputerCraft-<name>-<number>}, and belong to a group
+     * called {@code ComputerCraft-<name>} (which in turn will be a child group of the main {@code ComputerCraft} group.
+     *
+     * @param name The name for the thread group and child threads.
+     * @return The constructed thread factory.
+     * @see #builder(String)
+     */
+    public static ThreadFactory factory( String name )
+    {
+        return builder( name ).build();
+    }
+}