回答編集履歴

デッドロック回避

2021/12/10 09:43

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -42,13 +42,13 @@
 攻撃側が相手の wasAttacked を呼んでから hp を表示するまでの間に相手がこちらの wasAttacked を呼んでこちらの hp を減らし、その後こちらの表示が行われると、相手が攻撃した表示がされる前に hp が減っている状態になるからです。
-つまり、相手の hp を変更してから(相手の hp を減らしたことを)表示するまでの間で(同時に表示している)自分の hp が変更されてしまうのを防ぐ為にロックします。
+つまり、相手の hp を変更してから(相手の hp を減らしたことを)表示するまでの間で(同時に表示している)自分の hp が変更されてしまうのを防ぐ為にロックします。
-***　デッドロックします ***
+デッドロックが発生するため、 lock オブジェクトを static にしました。
-修正中です。ごめんなさい。
+lock の取り合いで負けた場合に倒される可能性があるため、ロック取得後に割り込みチェックを入れました。
@@ -62,11 +62,9 @@
   public static void main(String args[]) {
-    RPGCharacter c1 = null, c2 = null;
-    c1 = new RPGCharacter("Jay", 10);
+    RPGCharacter c1 = new RPGCharacter("Jay", 10);
-    c2 = new RPGCharacter("Rock", 10);
+    RPGCharacter c2 = new RPGCharacter("Rock", 10);
     c1.setTarget(c2);
@@ -84,12 +82,14 @@
 class RPGCharacter extends Thread {
+  private static Object[] lock = new Object[0];
   private String name;
   private int hp;
-  private Object[] lock = new Object[0];
   private RPGCharacter target;
@@ -122,7 +122,7 @@
         } catch(InterruptedException ie) {
-          break;
+          break; //倒された
         }
@@ -130,11 +130,13 @@
         synchronized(lock) {
+          if(interrupted()) break; //倒された
           boolean down = target.wasAttacked();
           System.out.println(name + " attacked " + target.name + " , " + this + " , " + target);
-          if(down) break;
+          if(down) break; //倒した
         }
@@ -164,11 +166,7 @@
   boolean wasAttacked() {
-    synchronized(lock) {
-      if(--hp > 0) return false; //まだ倒れない
+    if(--hp > 0) return false; //まだ倒れない
-    }
     interrupt();

デッドロックするので注意書き

2021/12/10 09:43

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -44,6 +44,14 @@
 つまり、相手の hp を変更してから(相手の hp を減らしたことを)表示するまでの間で(同時に表示している)自分の hp が変更されてしまうのを防ぐ為にロックします。
+***　デッドロックします ***
+修正中です。ごめんなさい。
 ```java
 package teratail_java.q373120;

修正・コード追加

2021/12/10 08:29

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -28,6 +28,200 @@
 相手が倒れたかを wasAttacked の戻り値で分かるようにすれば、攻撃した側も戦闘の終了が分かります。
-ですので while の条件で hp をチェックする必要もなく、 interrupted() だけで良いです。
+ですので while の条件で hp をチェックする必要もなく、 !interrupted() だけで良いです。
-そしてこの場合、ロックによる同期は必要ありません。
+~~そしてこの場合、ロックによる同期は必要ありません。~~
+----
+「ロックによる同期は必要ありません」と書きましたが、攻撃した後の表示で問題が有りそうですので、ロックを入れました。
+攻撃側が相手の wasAttacked を呼んでから hp を表示するまでの間に相手がこちらの wasAttacked を呼んでこちらの hp を減らし、その後こちらの表示が行われると、相手が攻撃した表示がされる前に hp が減っている状態になるからです。
+つまり、相手の hp を変更してから(相手の hp を減らしたことを)表示するまでの間で(同時に表示している)自分の hp が変更されてしまうのを防ぐ為にロックします。
+```java
+package teratail_java.q373120;
+public class Fight {
+  public static void main(String args[]) {
+    RPGCharacter c1 = null, c2 = null;
+    c1 = new RPGCharacter("Jay", 10);
+    c2 = new RPGCharacter("Rock", 10);
+    c1.setTarget(c2);
+    c2.setTarget(c1);
+    c1.start();
+    c2.start();
+  }
+}
+class RPGCharacter extends Thread {
+  private String name;
+  private int hp;
+  private Object[] lock = new Object[0];
+  private RPGCharacter target;
+  RPGCharacter(String name, int hp) {
+    this.name = name;
+    this.hp = hp;
+  }
+  void setTarget(RPGCharacter target) {
+    this.target = target;
+  }
+  void attack() {
+      while(!interrupted()) {
+        try {
+          Thread.sleep((long) (10 * Math.random()));
+        } catch(InterruptedException ie) {
+          break;
+        }
+        synchronized(lock) {
+          boolean down = target.wasAttacked();
+          System.out.println(name + " attacked " + target.name + " , " + this + " , " + target);
+          if(down) break;
+        }
+      }
+      if(hp <= 0) {
+        System.out.println(name + " lost.");
+      }
+  }
+  @Override
+  public void run() {
+    attack();
+  }
+  boolean wasAttacked() {
+    synchronized(lock) {
+      if(--hp > 0) return false; //まだ倒れない
+    }
+    interrupt();
+    return true; //倒れた
+  }
+  @Override
+  public String toString() {
+    return name + ":" + hp;
+  }
+}
+```
+```plain
+Jay attacked Rock , Jay:10 , Rock:9
+Rock attacked Jay , Rock:9 , Jay:9
+Jay attacked Rock , Jay:9 , Rock:8
+Jay attacked Rock , Jay:9 , Rock:7
+Rock attacked Jay , Rock:7 , Jay:8
+Jay attacked Rock , Jay:8 , Rock:6
+Rock attacked Jay , Rock:6 , Jay:7
+Jay attacked Rock , Jay:7 , Rock:5
+Jay attacked Rock , Jay:7 , Rock:4
+Rock attacked Jay , Rock:4 , Jay:6
+Jay attacked Rock , Jay:6 , Rock:3
+Rock attacked Jay , Rock:3 , Jay:5
+Rock attacked Jay , Rock:3 , Jay:4
+Jay attacked Rock , Jay:4 , Rock:2
+Rock attacked Jay , Rock:2 , Jay:3
+Jay attacked Rock , Jay:3 , Rock:1
+Rock attacked Jay , Rock:1 , Jay:2
+Rock attacked Jay , Rock:1 , Jay:1
+Rock attacked Jay , Rock:1 , Jay:0
+Jay lost.
+```

追記

2021/12/10 03:03

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -28,4 +28,6 @@
 相手が倒れたかを wasAttacked の戻り値で分かるようにすれば、攻撃した側も戦闘の終了が分かります。
+ですので while の条件で hp をチェックする必要もなく、 interrupted() だけで良いです。
 そしてこの場合、ロックによる同期は必要ありません。

追記

2021/12/10 02:00

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -26,4 +26,6 @@
 attacked メソッドは sleep が interrupted 例外を発したら while ループを抜けるようにすれば、 hp 0 で即スレッドは終了するでしょう。
+相手が倒れたかを wasAttacked の戻り値で分かるようにすれば、攻撃した側も戦闘の終了が分かります。
 そしてこの場合、ロックによる同期は必要ありません。

追記

2021/12/09 19:06

投稿

jimbe

スコア12659

test CHANGED Viewed

@@ -1 +1,29 @@
 ロックによる同期は、複数のスレッドが同じモノをロック・アンロックすることで機能するのであって、それぞれのスレッドがそれぞれのモノをロック・アンロックしていては、同期になりません。
+死んでいるのに攻撃してしまうのは、 while ループがそう書かれているからでしょう。
+while ループは
+・hpチェック
+・スリープ
+・攻撃
+を繰り返します。
+スリープ中に攻撃を受けて hp が 0 にされても、攻撃してからでなければチェックは行われません。
+もう少し言いますと、それぞれのキャラクターはそれぞれ自分の hp を管理すべきで、攻撃したからと言って相手の hp を減らす処理を攻撃した側が行うのは変ではないでしょうか。
+例えば「攻撃を受けた場合に相手から呼ばれる」wasAttacked メソッドを作り、 c1 が c2 を攻撃したなら c1 が c2 の wasAttacked を呼びます。
+wasAttacked は自身の hp を減らし、 0 になったら自分に interrupt を掛けます。
+attacked メソッドは sleep が interrupted 例外を発したら while ループを抜けるようにすれば、 hp 0 で即スレッドは終了するでしょう。
+そしてこの場合、ロックによる同期は必要ありません。