Uploaded image for project: 'Minecraft: Java Edition'
  1. Minecraft: Java Edition
  2. MC-119126

Pinging servers randomly fails on first attempt (multithread race condition)

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Duplicate
    • None
    • Minecraft 1.12
    • Windows 10 Pro 64-bit
      Minecraft Launcher bundled Java 1.8.0_u25
      AMD A10-4600M CPU
    • Confirmed

      The bug

      The first time the Multiplayer menu is loaded, one of the listed servers will almost always fail to ping, saying "Can't connect to server". However, upon pressing the Refresh button, it works just fine. This is an issue for servers because potential users will assume the server is down when it isn't.

      Code analysis (MCP names)

      I've analyzed the issue and found it's a race condition with the multi-threaded server pinger code. The server pinging code (net.minecraft.client.network.ServerPinger) is run in multiple threads, and each thread calls net.minecraft.network.NetworkManager.createNetworkManagerAndConnect(). This function uses "lazy loaders" (net.minecraft.util.LazyLoadBase) to initialize some static variables. The issue lies in net.minecraft.util.LazyLoadBase.getValue(). This function is not thread-safe. Below you'll find the code in question with comments explaining exactly what is happening and how to fix it.

      package net.minecraft.util;
      
      public abstract class LazyLoadBase<T>
      {
          private T value;
          private boolean isLoaded;
      
          public T getValue() // NOT THREAD-SAFE. SIMPLE FIX: declare the method synchronized
          {
              if (!this.isLoaded) // Thread 1: Condition is true; Thread 2: Condition is false
              {
                  this.isLoaded = true; // Thread 1: SETS isLoaded to true, BUT VALUE IS STILL NULL
                  this.value = this.load();
              }
      
              return this.value; // Thread 2: RETURNS NULL because this.load() is not finished in Thread 1
          }
      
          protected abstract T load();
      }
      

      Note that depending on the speed of the computer, this bug might not occur. If the first thread finishes quickly enough everything will appear normal.

            Unassigned Unassigned
            MWisBest Kyle Repinski
            Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved:
              CHK: