La synchronisation Java ne fonctionne pas comme prévu

donc selon le code que vous avez posté, vous n'obtiendriez jamais Broke Synchronization imprimé à moins que getBuffer() ne lance une exception entre le paramètre true et false . Voir un meilleur modèle ci-dessous.

Edit:

j'ai pris le code de @Luke et je l'ai réduit à cette classe de pastebin . Comme je le vois, @Luke frappe un bug de synchronisation JRE. Je sais que c'est dur à croire mais j'ai regardé le code et je ne vois pas le problème.

puisque vous mentionnez ConcurrentModificationException , je soupçonne que getBuffer() est le lancer quand il itère à travers le list . Le code que vous avez posté ne devrait jamais lancer un ConcurrentModificationException en raison de la synchronisation, mais je soupçonne qu'un code supplémentaire appelle add ou remove qui est et non synchronisé, ou vous supprimez pendant que vous itérez à travers le list . La seule façon de modifier une collection non synchronisée pendant que vous itérez à travers elle est par la Iterator.remove() méthode:

Iterator<Object> iterator = list.iterator();
while (iterator.hasNext()) {
   ...
   // it is ok to remove from the list this way while iterating
   iterator.remove();
}

Pour protéger votre drapeau, assurez-vous d'utiliser try/finally lorsque vous définissez une critique booléenne comme ça. Alors toute exception restaurerait le insideGetBuffer de manière appropriée:

synchronized public Object getBuffer() {
    insideGetBuffer = true;
    try {
        int i=0;
        for(Object item : list) {
            i++;
        }
    } finally {
        insideGetBuffer = false;
    }
    return null;
}

Aussi, il est un meilleur modèle pour synchroniser autour d'un objet particulier au lieu d'utiliser la synchronisation de méthode. Si vous essayez de protéger le list , alors ajouter la synchronisation autour de cette liste à chaque fois serait mieux.n

 synchronized (list) {
    list.remove();
 }

vous pouvez également transformer votre liste en une liste synchronisée que vous n'auriez pas à synchronize à chaque fois:

 List<Object> list = Collections.synchronizedList(new ArrayList<Object>());

D'après ce code, il n'y a que deux façons d'imprimer" Broke Synchronization".

1. Ils sont la synchronisation sur les différents objets (dont vous dites qu'ils ne le sont pas)
2. le insideGetBuffer est modifié par un autre fil à l'extérieur du bloc synchronisé.

sans ces deux-là, il n'y a pas moyen que le code que vous avez indiqué imprime "broke Synchronization" et le ConcurrentModificationException . Pouvez-vous donner un petit extrait de code qui peut être exécuté à prouver ce que vous dites?

mise à jour:

J'ai parcouru L'exemple que Luke a posté et je vois des comportements étranges sur les fenêtres Java 1.6_24-64 bits. La même instance de TestBuffer et la valeur du insideGetBuffer est "alternée" à l'intérieur de la méthode remove. Note ce champ n'est pas mis à jour à l'extérieur d'une région synchronisée. Il n'y a qu'une seule instance de TestBuffer mais supposons qu'ils ne soient pas - insideGetBuffer ne serait jamais défini à true (donc ce doit être la même instance).

    synchronized public void remove(Object item) {

            boolean b = insideGetBuffer;
            if(insideGetBuffer){
                    System.out.println("Broke Synchronization : " +  b + " - " + insideGetBuffer);
            }
    }

parfois il imprime Broke Synchronization : true - false

je travaille à faire tourner l'assembleur sur Windows 64 bit Java.

une exception de modification simultanée, la plupart du temps, n'est pas causée par des threads concurrents. Il est causé par la modification de la collection alors qu'il est itéré:

for (Object item : list) {
    if (someCondition) {
         list.remove(item);
    }
}

le code ci-dessus causerait une exception de modification Concurrentemodificationexception si une condition est vraie. Pendant l'itération, la collection ne peut être modifiée que par les méthodes de l'itérateur:

for (Iterator<Object> it = list.iterator(); it.hasNext(); ) {
    Object item = it.next();
    if (someCondition) {
         it.remove();
    }
}

je soupçonne que c'est ce qui se passe dans votre vrai code. Le le code affiché est bon.

pouvez-vous essayer ce code qui est un test autonome?

public static class TestBuffer {
    private final List<Object> list = new ArrayList<Object>();
    private boolean insideGetBuffer = false;

    public TestBuffer() {
        System.out.println("Creating a TestBuffer");
    }

    synchronized public void add(Object item) {
        list.add(item);
    }

    synchronized public void remove(Object item) {
        if (insideGetBuffer) {
            System.out.println("Broke Synchronization ");
            System.out.println(item);
        }

        list.remove(item);
    }

    synchronized public void getBuffer() {
        insideGetBuffer = true;
//      System.out.println("getBuffer.");
        try {
            int count = 0;
            for (int i = 0, listSize = list.size(); i < listSize; i++) {
                if (list.get(i) != null)
                    count++;
            }
        } finally {
//          System.out.println(".getBuffer");
            insideGetBuffer = false;
        }
    }
}

public static void main(String... args) throws IOException {
    final TestBuffer tb = new TestBuffer();
    ExecutorService service = Executors.newCachedThreadPool();
    final AtomicLong count = new AtomicLong();
    for (int i = 0; i < 16; i++) {
        final int finalI = i;
        service.submit(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    for (int j = 0; j < 1000000; j++) {
                        tb.add(finalI);
                        tb.getBuffer();
                        tb.remove(finalI);
                    }
                    System.out.printf("%d,: %,d%n", finalI, count.addAndGet(1000000));
                }
            }
        });
    }
}

imprime

Creating a TestBuffer
11,: 1,000,000
2,: 2,000,000
... many deleted ...
2,: 100,000,000
1,: 101,000,000

regarder votre trace de pile plus en détail.

Caused by: java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
    at java.util.HashMap$KeyIterator.next(Unknown Source)
    at <removed>.getBuffer(<removed>.java:62)

vous pouvez voir que vous accédez au jeu de clés d'un HashMap, pas à une liste. C'est important parce que l'ensemble de clés est un vue sur la carte sous-jacente. Cela signifie que vous devez vous assurer que tous les accès cette carte est également protégée par la même serrure. par exemple, dites que vous avez un setter comme

Collection list;
public void setList(Collection list) { this.list = list; }


// somewhere else
Map map = new HashMap();
obj.setList(map.keySet());

// "list" is accessed in another thread which is locked by this thread does this
map.put("hello", "world");
// now an Iterator in another thread on list is invalid.

'getBuffer' de la classe Controller crée ce problème. Si deux threads entrent en même temps dans la condition suivante " si " pour la première fois, le controller finira par créer deux objets buffer. ajouter une fonction est appelée sur le premier objet et l'enlever sera appelé sur le deuxième objet.

if (colorToBufferMap.containsKey(defaultColor)) {

lorsque deux threads (ajouter et supprimer des threads) entrent en même temps (lorsque le tampon n'a pas encore été ajouté à colorToBufferMap), ci-dessus si statement retournera false et les deux threads entreront la partie else et créeront deux tampons, puisque buffer est une variable locale ces deux threads recevront deux instances différentes de buffer dans le cadre de l'instruction return. Cependant, seul le dernier créé sera stocké dans la variable globale 'colorToBufferMap'.

au-dessus de la ligne problématique fait partie de la fonction getBuffer

public TestBuffer getBuffer() {
    TestBuffer buffer = null;
    if (colorToBufferMap.containsKey(defaultColor)) {
        buffer = colorToBufferMap.get(defaultColor);
    } else {
        buffer = new TestBuffer();
        colorToBufferMap.put(defaultColor, buffer);
    }
    return buffer;
}

synchronisation de la fonction "getBuffer" dans la classe contrôleur va résoudre ce problème.

Edit: Réponse valide uniquement lorsque les deux différentes instances de l'Objet sont utilisés en appelant la méthode à plusieurs reprises.

le scénario: Vous avez deux méthodes synchronisées. Une pour supprimer une entité et une autre pour accéder. Le problème vient quand 1 thread est à l'intérieur de la méthode remove et un autre thread est dans la méthode getBuffer et définit le insideGetBuffer = true.

comme vous avez découvert que vous devez mettre la synchronisation sur la liste parce que ces deux méthodes fonctionnent sur vous liste.

si l'accès à list et insideGetBuffer est entièrement contenu dans ce code, le code semble certainement thread sûr et je ne vois pas une possibilité de" synchronisation cassée " peut être imprimé, sauf un bug JVM.

pouvez-vous vérifier deux fois tous les accès possibles à vos variables membres (list et insideGetBuffer)? Les possibilités comprennent si la liste vous a été transmise par le constructeur (ce que votre code ne montre pas) ou si ces variables sont des variables protégées afin que les sous-classes puissent les changer.

une autre possibilité est l'accès par réflexion.

Je ne crois pas que ce soit un bug dans le JVM.

mon premier soupçon était qu'il s'agissait d'une sorte d'opération de réorganisation que le compilateur fait (sur ma machine, il fonctionne bien dans un débogueur, mais la synchronisation échoue lors de l'exécution) mais

Je ne peux pas vous dire pourquoi, mais je soupçonne fortement que quelque chose abandonne le verrouillage sur TestBuffer qui est implicite en déclarant getBuffer() et remove(...) synchronis.

par exemple, remplacez-les par ceci:

public void getBuffer() {
    synchronized (this) {
        this.insideGetBuffer = true;
        try {
            int i = 0;
            for (Object item : this.list) {
                if (item != null) {
                    i++;
                }
            }
        } finally {
            this.insideGetBuffer = false;
        }
    }

}

public void remove(final Object item) {
    synchronized (this) {
        // fails if this is called while getBuffer is running
        if (this.insideGetBuffer) {
            System.out.println("Broke Synchronization ");
            System.out.println(item);
        }
    }
}

Et vous avez toujours votre erreur de synchronisation. Mais choisissez autre chose pour vous connecter, par exemple:

private Object lock = new Object();
public void getBuffer() {
    synchronized (this.lock) {
        this.insideGetBuffer = true;
        try {
            int i = 0;
            for (Object item : this.list) {
                if (item != null) {
                    i++;
                }
            }
        } finally {
            this.insideGetBuffer = false;
        }
    }

}

public void remove(final Object item) {
    synchronized (this.lock) {
        // fails if this is called while getBuffer is running
        if (this.insideGetBuffer) {
            System.out.println("Broke Synchronization ");
            System.out.println(item);
        }
    }
}

Et tout fonctionne comme prévu.

maintenant, vous pouvez simuler l'abandon de la serrure en ajoutant:

this.lock.wait(1);

dans la boucle for de getBuffer() et vous allez recommencer à échouer.

je reste perplexe sur ce est de donner la verrouillage, mais en général, il pourrait être une meilleure idée d'utiliser la synchronisation explicite sur les serrures protégées que l'opérateur de synchronisation.

j'ai déjà eu un problème similaire. L'erreur est que vous n'avez pas déclaré certains domaine comme volatile . Ce mot-clé est utilisé pour indiquer qu'un champ sera modifié par différents threads, et donc il ne peut pas être mis en cache. Au lieu de cela, toutes les Écritures et les lectures doivent aller à l'emplacement "réel" de la mémoire du champ.

pour plus d'information, juste google pour " Java Memory Model

bien que la plupart des lecteurs se concentrent sur la classe TestBuffer , je pense que le problème pourrait être ailleurs (par exemple, Avez-vous essayé d'ajouter la syncronisation sur le contrôleur de classe? Ou rendre volatiles ses champs?).

ps. notez que différentes VM java peuvent utiliser une optimisation différente dans différentes plates-formes, et donc les problèmes de synchronisation peuvent apparaître plus souvent sur une plate-forme qu'une autre. La seule façon d'être sûr est d'être conforme aux spécifications de Java, et de déposer un bogue si une VM ne le respecte pas.