Veo algunas NPE extrañas y descubrí que se trata de un problema de concurrencia. Estoy usando un código similar al siguiente:

class Manager {
  private final ConcurrentMap<String, Value> map = new ConcurrentHashMap<>();

  public Value get(String key) {
    Value v = map.get(key);
    if (v == null) {
      new Value(key, this);
      v = map.get(key);
    }
    return v;
  }

  public void doIt(String key) {
    get(key).doIt();
  }

  void register(String key, Value v) {
    map.put(key, v);
  }
}

class Value {
  private final Value[] values;
  private final SubValue v;

  Value(String key, Manager m) {
    m.register(key, this);

    // Initialize some values, this is where a cycle can be introduced
    // This is just some sample code, the gist is, we call Manager.get
    this.vs = new Value[]{ m.get("some-other-key") };
    // Other code ...

    this.v = new SubValue(m);
  }

  public void doIt() {
    this.v.doIt(); // <--- NPE here. v is null sometimes
  }
}

Cuando llamo a Manager.doIt, a veces obtengo una NPE porque Value.v es null. Por lo que entiendo las relaciones sucede antes, es posible que cuando se llame a Manager.get al mismo tiempo y cuando aún no haya una entrada para una clave, pueda recuperar un valor aún no inicializado por completo.

Estoy registrando objetos en el constructor de Value porque el gráfico de objetos entre Value objetos puede tener ciclos y sin esto, obtendría una excepción de stackoverflow.

La pregunta ahora es, ¿cómo me aseguro en doIt de que Value y todos los valores conectados estén completamente inicializados? Estoy pensando en hacer una especie de bloqueo de verificación doble en Manager.get pero no estoy seguro de cómo resolver esto mejor. Algo como esto:

  public Value get(String key) {
    Value v = map.get(key);
    if (v == null) {
      synchronized(map) {
        v = map.get(key);
        if (v == null) {
          v = new Value(key, this);
        }
      }
    }
    return v;
  }

¿Alguien tiene una mejor idea sobre cómo resolver esto o ve un problema de concurrencia con ese código?

1
Christian Beikov 7 oct. 2020 a las 20:02

2 respuestas

La mejor respuesta

La solución con la que fui es la siguiente, que es escalable y, por lo que puedo decir, segura:

class Manager {
  private final ConcurrentMap<String, Value> map = new ConcurrentHashMap<>();

  public Value get(String key) {
    Value v = map.get(key);
    if (v == null) {
      Map<String, Value> subMap = new HashMap<>();
      new Value(key, subMap);
      map.putAll(subMap);
      v = map.get(key);
    }
    return v;
  }

  public void doIt(String key) {
    get(key).doIt();
  }
}

class Value {
  private final Value[] values;
  private final SubValue v;

  Value(String key, Map<String, Value> subMap) {
    subMap.put(key, this);

    // Initialize some values, this is where a cycle can be introduced
    // This is just some sample code, the gist is, we call Manager.get
    this.vs = new Value[]{ subMap.containsKey("some-other-key") ? subMap.get("some-other-key") : m.get("some-other-key") };
    // Other code ...

    this.v = new SubValue(m);
  }

  public void doIt() {
    this.v.doIt();
  }
}
0
Christian Beikov 9 oct. 2020 a las 10:17

El problema aquí es que estás haciendo que this escape en el constructor.

class Value {
  private final Value[] values;
  private final SubValue v;

  Value(String key, Manager m) {
    m.register(key, this); <--- (this is not properly constructed)

    // Initialize some values, this is where a cycle can be introduced
    // This is just some sample code, the gist is, we call Manager.get
    this.vs = new Value[]{ m.get("some-other-key") };
    // Other code ...

    this.v = new SubValue(m);
  }

  public void doIt() {
    this.v.doIt(); // <--- NPE here. v is null sometimes
  }
}

Ahora, si algún hilo llama a doIt en una clave que tiene un objeto construido incorrectamente en el mapa, puede obtener un NPE ya que la Subvalue v para ese objeto podría no haberse inicializado .

El código tiene otro problema. Manager.get() es una acción compuesta y debe encapsularse en un bloque synchronised. Si un subproceso observa un valor null para una clave, en el momento en que ingresa al bloque if, esa observación puede volverse obsoleta. Dado que el mapa está involucrado en la acción compuesta, todos los métodos que hacen referencia al mapa deben estar protegidos por el mismo bloqueo; básicamente, es necesario proteger get() y register() con el mismo bloqueo.

1
Prashant Pandey 7 oct. 2020 a las 20:06