2009-11-27 8 views
28

I ont un code Java multithread, dans lequel:synchronisation sur les variables locales

  • plusieurs fils lire des objets avec état de la mémoire partagée synchronisé (à savoir à la suite de cela, certains fils peuvent référencer les mêmes objets); Chaque thread appelle alors une méthode process() et passe son objet là;
  • process() traite les objets d'une manière ou d'une autre, ce qui peut entraîner la modification de l'état des objets;
  • ces modifications d'état doivent être synchronisées.

J'ai créé une méthode comme ça:

public void process(Foo[] foos) { 
    for (final Foo foo : foos) { 
     if (foo.needsProcessing()) { 
      synchronized (foo) { 
       foo.process(); // foo's state may be changed here 
      } 
     } 
    } 
} 

Au meilleur de ma connaissance, cela semble légitime. Cependant, l'inspection d'IntelliJ se plaint de la synchronisation sur la variable locale parce que "les différents threads sont très susceptibles d'avoir différentes instances locales" (pas valide pour moi, car je suis pas initialisation des foos dans la méthode). Essentiellement, ce que je veux réaliser ici est le même que d'avoir la méthode Foo.process() synchronisée (ce qui n'est pas une option pour moi, car Foo fait partie de la bibliothèque 3rd party). Je me suis habitué au code sans marques jaunes, donc tout conseil de la communauté est apprécié. Est-ce vraiment si mauvais de faire la synchronisation sur les locaux? Y a-t-il une alternative qui fonctionnera dans ma situation?

Merci d'avance!

Répondre

21
if (foo.needsProcessing()) { 
    synchronized (foo) { 
     foo.process(); // foo's state may be changed here 
    } 
} 

Je pense qu'il ya une condition de course dans le fragment ci-dessus qui pourrait entraîner foo.process() étant parfois appelé deux fois sur le même objet. Il devrait être:

synchronized (foo) { 
    if (foo.needsProcessing()) { 
     foo.process(); // foo's state may be changed here 
    } 
} 

Est-ce vraiment si mal que ça pour se synchroniser sur la population locale?

Il n'est pas mauvais de synchroniser sur les locales en soi. Les vrais problèmes sont les suivants:

  • si les différents threads se synchronisent sur les bons objets pour obtenir une bonne synchronisation et

  • si quelque chose d'autre pourrait causer des problèmes en synchronisant sur ces objets.

+1

Droit, merci! BTW, que pensez-vous de la question principale ici? Est-ce vraiment si mauvais de se synchroniser sur les locaux? –

1

Refactoriser le corps de la boucle en une méthode séparée en prenant foo comme paramètre.

+0

Je devrai me synchroniser sur le paramètre de cette méthode aussi –

+0

Oui, mais ce ne sera pas une variable locale. –

+2

IntelliJ a un autre type d'inspection "les différents threads sont très susceptibles d'avoir des paramètres de méthode différents" :) –

1

Aucune auto-intelligence n'est parfaite. Je pense que votre idée de la synchronisation sur la variable locale est tout à fait correcte. Alors faites peut-être votre chemin (ce qui est juste) et suggérez à JetBrains d'affiner son inspection.

0

Votre synchronisation sur un objet dans une collection ne présente aucun problème. Vous pouvez essayer de remplacer le foreach avec une normale boucle:

for (int n = 0; n < Foos.length; n++) { 
    Foo foo = Foos[n]; 

    if (null != foo && foo.needsProcessing()) { 
     synchronized (foo) { 
      foo.process(); // foo's state may be changed here 
     } 
    } 
} 

ou même (si le détecteur ne se déclenche pas sur le Foo foo):

for (int n = 0; n < foos.length; n++) { 
    if (null != foos[n] && foos[n].needsProcessing()) { 
     synchronized (foos[n]) { 
      foos[n].process(); // foos[n]'s state may be changed here 
     } 
    } 
} 

Ne pas utiliser une valeur temporaire pour empêcher le multiple foos[n] n'est pas la meilleure pratique, mais si cela fonctionne pour éviter l'avertissement indésirable, vous pourriez vivre avec. Ajouter un commentaire pourquoi ce code a une forme déviante. :-)

2

IDE est censé vous aider, s'il fait une erreur, vous ne devriez pas vous pencher et vous plaire.

Vous pouvez désactiver cette inspection dans IntelliJ. (C'est le seul qui soit activé par défaut sous "Threading Issues".) Est-ce l'erreur la plus répandue?)

0

Dans le monde .NET, les objets portent parfois leur objet lock en tant que propriété.

synchronized (foo.getSyncRoot()) { 
    if (foo.needsProcessing()) { 
     foo.process(); // foo's state may be changed here 
    } 
} 

Ceci permet à l'objet de donner un objet de verrouillage différent, en fonction de sa mise en œuvre (par exemple déléguer le moniteur à une base de données sous-jacente de connexion ou quelque chose).

4

La réponse de Stephen C a des problèmes, il est saisie d'un grand nombre de verrous de synchronisation inutilement, assez curieusement la meilleure façon de formater est:

public void process(Foo[] foos) { 
     for (final Foo foo : foos) { 
      if (foo.needsProcessing()) { 
       synchronized (foo) { 
        if (foo.needsProcessing()) { 
         foo.process(); // foo's state may be changed here 
        } 
       } 
      } 
     } 
    } 

Obtenir le verrou de synchronisation peut parfois prendre un certain temps et si elle a eu lieu quelque chose changeait quelque chose. Cela pourrait être quelque chose qui a changé le statut de traitement nécessaire de foo dans ce temps.

Vous ne voulez pas attendre le verrou si vous n'avez pas besoin de traiter l'objet. Et après avoir obtenu le verrou, il n'a peut-être pas besoin de traitement. Donc, même si cela semble un peu bête et les programmeurs novices pourraient être enclins à supprimer l'un des contrôles, c'est en fait la bonne façon de faire cela tant que foo.needsProcessing() est une fonction négligeable.


Apporter ce retour à la question principale, quand il est le cas que vous souhaitez synchroniser en fonction des valeurs locales dans un tableau, il est parce que le temps est essentiel. Et dans ces cas, la dernière chose que vous voulez faire est de verrouiller chaque élément dans le tableau ou de traiter les données deux fois. Vous ne feriez que synchroniser des objets locaux si vous avez quelques threads qui font beaucoup de travail et que vous devriez très rarement toucher les mêmes données mais très bien. Faire cela ne touchera le verrou que si et seulement si le traitement de ce foo qui nécessite un traitement entraînerait des erreurs de simultanéité. Vous aurez essentiellement besoin d'une syntaxe double gated dans les cas où vous voulez synchroniser en fonction des seuls objets précis dans le tableau en tant que tel. Ceci empêche le double traitement des foos et le verrouillage de tout foo n'ayant pas besoin d'être traité. Vous avez le cas très rare de bloquer le thread ou même d'entrer un verrou seulement et exactement quand c'est important, et votre thread n'est bloqué que là où le blocage ne conduirait pas à des erreurs de simultanéité.

Questions connexes