Avoid early initializing Netty

Today when we load the Netty plugins, we indirectly cause several Netty
classes to initialize. This is because we attempt to load some classes
by name, and loading these classes is done in a way that triggers a long
chain of class initializers within Netty. We should not do this, this
can lead to log messages before the logger is loader, and it leads to
initialization in cases when the classes would never be needed (for
example, Netty 3 class initialization is never needed if Netty 4 is
used, and vice versa). This commit avoids this early initialization of
these classes by removing the need for the early loading.

Relates #19819

client/transport/src/main/java/org/elasticsearch/transport/client/PreBuiltTransportClient.java

@@ -21,7 +21,6 @@ package org.elasticsearch.transport.client;
 
 import io.netty.util.ThreadDeathWatcher;
 import io.netty.util.concurrent.GlobalEventExecutor;
-import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.client.transport.TransportClient;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Setting;
@@ -57,7 +56,6 @@ public class PreBuiltTransportClient extends TransportClient {
                     Arrays.asList(
                             Netty3Plugin.class,
                             Netty4Plugin.class,
-                            TransportPlugin.class,
                             ReindexPlugin.class,
                             PercolatorPlugin.class,
                             MustachePlugin.class));
@@ -71,24 +69,6 @@ public class PreBuiltTransportClient extends TransportClient {
         super(settings, Settings.EMPTY, addPlugins(plugins, PRE_INSTALLED_PLUGINS));
     }
 
-    public static final class TransportPlugin extends Plugin {
-
-        private static final Setting<Boolean> ASSERT_NETTY_BUGLEVEL =
-                Setting.boolSetting("netty.assert.buglevel", true, Setting.Property.NodeScope);
-
-        @Override
-        public List<Setting<?>> getSettings() {
-            return Collections.singletonList(ASSERT_NETTY_BUGLEVEL);
-        }
-
-        @Override
-        public Settings additionalSettings() {
-            return Settings.builder().put("netty.assert.buglevel", true)
-                    .build();
-        }
-
-    }
-
     @Override
     public void close() {
         super.close();

modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java

@@ -60,17 +60,16 @@ public class ReindexFromRemoteWithAuthTests extends ESSingleNodeTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> getPlugins() {
-        return Arrays.asList(RetryTests.BogusPlugin.class,
-                Netty4Plugin.class,
-                ReindexFromRemoteWithAuthTests.TestPlugin.class,
-                ReindexPlugin.class);
+        return Arrays.asList(
+            Netty4Plugin.class,
+            ReindexFromRemoteWithAuthTests.TestPlugin.class,
+            ReindexPlugin.class);
     }
 
     @Override
     protected Settings nodeSettings() {
         Settings.Builder settings = Settings.builder().put(super.nodeSettings());
         // Weird incantation required to test with netty
-        settings.put("netty.assert.buglevel", false);
         settings.put(NetworkModule.HTTP_ENABLED.getKey(), true);
         // Whitelist reindexing from the http host we're going to use
         settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "myself");

modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java

@@ -93,20 +93,7 @@ public class RetryTests extends ESSingleNodeTestCase {
         return pluginList(
                 ReindexPlugin.class,
                 Netty3Plugin.class,
-                Netty4Plugin.class,
-                BogusPlugin.class);
-    }
-
-    public static final class BogusPlugin extends Plugin {
-        // this runs without the permission from the netty module so it will fail since reindex can't set the property
-        // to make it still work we disable that check but need to register the setting first
-        private static final Setting<Boolean> ASSERT_NETTY_BUGLEVEL = Setting.boolSetting("netty.assert.buglevel", true,
-            Setting.Property.NodeScope);
-
-        @Override
-        public List<Setting<?>> getSettings() {
-            return Collections.singletonList(ASSERT_NETTY_BUGLEVEL);
-        }
+                Netty4Plugin.class);
     }
 
     /**
@@ -115,7 +102,6 @@ public class RetryTests extends ESSingleNodeTestCase {
     @Override
     protected Settings nodeSettings() {
         Settings.Builder settings = Settings.builder().put(super.nodeSettings());
-        settings.put("netty.assert.buglevel", false);
         // Use pools of size 1 so we can block them
         settings.put("thread_pool.bulk.size", 1);
         settings.put("thread_pool.search.size", 1);

modules/transport-netty3/src/main/java/org/elasticsearch/transport/Netty3Plugin.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.transport;
 
-import org.elasticsearch.SpecialPermission;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -36,31 +35,6 @@ public class Netty3Plugin extends Plugin {
     public static final String NETTY_TRANSPORT_NAME = "netty3";
     public static final String NETTY_HTTP_TRANSPORT_NAME = "netty3";
 
-    public Netty3Plugin(Settings settings) {
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(new SpecialPermission());
-        }
-        AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
-            try {
-                Class.forName("org.jboss.netty.channel.socket.nio.SelectorUtil");
-            } catch (ClassNotFoundException e) {
-                throw new AssertionError(e); // we don't do anything with this
-            }
-            return null;
-        });
-        /*
-         * Asserts that sun.nio.ch.bugLevel has been set to a non-null value. This assertion will fail if the corresponding code
-         * is not executed in a doPrivileged block. This can be disabled via `netty.assert.buglevel` setting which isn't registered
-         * by default but test can do so if they depend on the jar instead of the module.
-         */
-        //TODO Once we have no jar level dependency we can get rid of this.
-        if (settings.getAsBoolean("netty.assert.buglevel", true)) {
-            assert System.getProperty("sun.nio.ch.bugLevel") != null :
-                "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
-        }
-    }
-
     @Override
     public List<Setting<?>> getSettings() {
         return Arrays.asList(
@@ -89,4 +63,5 @@ public class Netty3Plugin extends Plugin {
         }
         networkModule.registerTransport(NETTY_TRANSPORT_NAME, Netty3Transport.class);
     }
+
 }

modules/transport-netty3/src/main/java/org/elasticsearch/transport/netty3/Netty3Utils.java

@@ -20,6 +20,7 @@ package org.elasticsearch.transport.netty3;
 
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefIterator;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.logging.Loggers;
 import org.jboss.netty.buffer.ChannelBuffer;
@@ -30,6 +31,8 @@ import org.jboss.netty.util.ThreadNameDeterminer;
 import org.jboss.netty.util.ThreadRenamingRunnable;
 
 import java.io.IOException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.ArrayList;
 
 /**
@@ -102,6 +105,25 @@ public class Netty3Utils {
         });
 
         ThreadRenamingRunnable.setThreadNameDeterminer(ES_THREAD_NAME_DETERMINER);
+
+        // Netty 3 SelectorUtil wants to set this; however, it does not execute the property write
+        // in a privileged block so we just do what Netty wants to do here
+        final String key = "sun.nio.ch.bugLevel";
+        final String buglevel = System.getProperty(key);
+        if (buglevel == null) {
+            try {
+                AccessController.doPrivileged(new PrivilegedAction<Void>() {
+                    @Override
+                    @SuppressForbidden(reason = "to use System#setProperty to set sun.nio.ch.bugLevel")
+                    public Void run() {
+                        System.setProperty(key, "");
+                        return null;
+                    }
+                });
+            } catch (final SecurityException e) {
+                Loggers.getLogger(Netty3Utils.class).debug("Unable to get/set System Property: {}", e, key);
+            }
+        }
     }
 
     public static void setup() {

modules/transport-netty4/src/main/java/org/elasticsearch/transport/Netty4Plugin.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.transport;
 
-import org.elasticsearch.SpecialPermission;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -27,8 +26,6 @@ import org.elasticsearch.http.netty4.Netty4HttpServerTransport;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.transport.netty4.Netty4Transport;
 
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.List;
 
@@ -37,31 +34,6 @@ public class Netty4Plugin extends Plugin {
     public static final String NETTY_TRANSPORT_NAME = "netty4";
     public static final String NETTY_HTTP_TRANSPORT_NAME = "netty4";
 
-    public Netty4Plugin(Settings settings) {
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(new SpecialPermission());
-        }
-        AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
-            try {
-                Class.forName("io.netty.channel.nio.NioEventLoop");
-            } catch (ClassNotFoundException e) {
-                throw new AssertionError(e); // we don't do anything with this
-            }
-            return null;
-        });
-        /*
-         * Asserts that sun.nio.ch.bugLevel has been set to a non-null value. This assertion will fail if the corresponding code
-         * is not executed in a doPrivileged block. This can be disabled via `netty.assert.buglevel` setting which isn't registered
-         * by default but test can do so if they depend on the jar instead of the module.
-         */
-        //TODO Once we have no jar level dependency we can get rid of this.
-        if (settings.getAsBoolean("netty.assert.buglevel", true)) {
-            assert System.getProperty("sun.nio.ch.bugLevel") != null :
-                "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
-        }
-    }
-
     @Override
     public List<Setting<?>> getSettings() {
         return Arrays.asList(

modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy

@@ -17,8 +17,8 @@
  * under the License.
  */
 
-grant {
-   // Netty SelectorUtil wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854
+grant codeBase "${codebase.netty-transport-4.1.4.Final.jar}" {
+   // Netty NioEventLoop wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854
    // the bug says it only happened rarely, and that its fixed, but apparently it still happens rarely!
    permission java.util.PropertyPermission "sun.nio.ch.bugLevel", "write";
 };

modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/Netty4TransportPublishAddressIT.java

@@ -28,7 +28,6 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.BoundTransportAddress;
 import org.elasticsearch.common.transport.InetSocketTransportAddress;
 import org.elasticsearch.common.transport.TransportAddress;
-import org.elasticsearch.node.Node;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.transport.Netty4Plugin;
 

qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java

@@ -50,7 +50,6 @@ import java.util.Locale;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomAsciiOfLength;
-import static com.carrotsearch.randomizedtesting.RandomizedTest.randomBoolean;
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomIntBetween;
 import static org.hamcrest.Matchers.notNullValue;
 

qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpSmokeTestCase.java

@@ -61,7 +61,6 @@ public abstract class HttpSmokeTestCase extends ESIntegTestCase {
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-                .put("netty.assert.buglevel", false)
                 .put(NetworkModule.TRANSPORT_TYPE_KEY, nodeTransportTypeKey)
                 .put(NetworkModule.HTTP_TYPE_KEY, nodeHttpTypeKey)
                 .put(NetworkModule.HTTP_ENABLED.getKey(), true).build();
@@ -69,19 +68,18 @@ public abstract class HttpSmokeTestCase extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return pluginList(MockTcpTransportPlugin.class, Netty3Plugin.class, Netty4Plugin.class, BogusPlugin.class);
+        return pluginList(MockTcpTransportPlugin.class, Netty3Plugin.class, Netty4Plugin.class);
     }
 
     @Override
     protected Collection<Class<? extends Plugin>> transportClientPlugins() {
-        return pluginList(MockTcpTransportPlugin.class, Netty3Plugin.class, Netty4Plugin.class, BogusPlugin.class);
+        return pluginList(MockTcpTransportPlugin.class, Netty3Plugin.class, Netty4Plugin.class);
     }
 
     @Override
     protected Settings transportClientSettings() {
         return Settings.builder()
                 .put(super.transportClientSettings())
-                .put("netty.assert.buglevel", false)
                 .put(NetworkModule.TRANSPORT_TYPE_KEY, clientTypeKey)
                 .build();
     }
@@ -91,18 +89,4 @@ public abstract class HttpSmokeTestCase extends ESIntegTestCase {
         return true;
     }
 
-    public static final class BogusPlugin extends Plugin {
-
-        // this runs without the permission from the netty modules so it will fail since reindex can't set the property
-        // to make it still work we disable that check but need to register the setting first
-        private static final Setting<Boolean> ASSERT_NETTY_BUGLEVEL =
-                Setting.boolSetting("netty.assert.buglevel", true, Setting.Property.NodeScope);
-
-        @Override
-        public List<Setting<?>> getSettings() {
-            return Collections.singletonList(ASSERT_NETTY_BUGLEVEL);
-        }
-
-    }
-
 }