J'ai récemment appris que les constructeurs n'étaient pas thread-safe en Java, alors que j'avais toujours cru le contraire, notamment parce qu'on ne peut pas les « synchronized ». Pour ceux qui ne verraient pas de quoi je parle, voici un extrait du livre Java Concurrency in Practice (très bon livre au passage) :

Code : Sélectionner tout - Visualiser dans une fenêtre à part
1
2
3
4
5
6
7
8
9
10
11
12
13
14
class Holder {
 
	private int n;
 
	public Holder(int n) {
		this.n = n;
	}
 
	public void assertSanity() {
		if (n != n) {
			throw new AssertionError("This statement is false.");
		}
	}
}
Si la méthode assertSanity est appelée dans un autre thread que celui ayant créé l'instance, l'exception AssertionError peut très bien être levée. Par exemple :

Code : Sélectionner tout - Visualiser dans une fenêtre à part
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
private static class HolderThread implements Runnable {
 
	// Le final importe !
	private final Holder holder;
 
	public HolderThread(Holder holder) {
		this.holder = holder;
	}
 
	@Override
	public void run() {
		holder.assertSanity();
	}
}
 
// Code susceptible de lever une AssertionError.
new Thread(new HolderThread(new Holder(10))).start();
Ça ne se produira pas forcément sur toutes les JVM, mais ça pourra au moins se produire sur certaines. Je précise au passage que la JSR 133 relative à la révision du modèle mémoire de Java ne change pas ce comportement. Si j'ai bien compris, pour résoudre le problème il faudrait adopter l'une des approches suivantes :

Code : Sélectionner tout - Visualiser dans une fenêtre à part
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
public class TestThreadSafety {
 
	public interface Holder {
 
		public void assertSanity();
	}
 
	@NotThreadSafe
	public static class MutableHolder implements Holder {
 
		private int n;
 
		public MutableHolder(int n) {
			this.n = n;
		}
 
		@Override
		public void assertSanity() {
			if (n != n) {
				throw new AssertionError("This statement is false.");
			}
		}
	}
 
	@ThreadSafe
	public static class FinalHolder implements Holder {
 
		private final int n;
 
		public FinalHolder(int n) {
			this.n = n;
		}
 
		@Override
		public void assertSanity() {
			if (n != n) {
				throw new AssertionError("This statement is false.");
			}
		}
	}
 
	@ThreadSafe
	public static class SynchronizedSafeHolder implements Holder {
 
		private int n;
 
		public SynchronizedSafeHolder(int n) {
			synchronized (this) {
				this.n = n;
			}
		}
 
		@Override
		public synchronized void assertSanity() {
			if (n != n) {
				throw new AssertionError("This statement is false.");
			}
		}
	}
 
	@ThreadSafe
	public static class VolatileSafeHolder implements Holder {
 
		private volatile int n; // atomique
 
		public VolatileSafeHolder(int n) {
			synchronized (this) {
				this.n = n;
			}
		}
 
		@Override
		public synchronized void assertSanity() {
			if (n != n) {
				throw new AssertionError("This statement is false.");
			}
		}
	}
 
	@ThreadSafe
	public static class AtomicSafeHolder implements Holder {
 
		// Le final importe !
		private final AtomicInteger n = new AtomicInteger(10);
 
		public AtomicSafeHolder(int n) {
			this.n.set(n);
		}
 
		@Override
		public synchronized void assertSanity() {
			if (n.get() != n.get()) {
				throw new AssertionError("This statement is false.");
			}
		}
	}
 
	private static class HolderThread implements Runnable {
 
		// Le final importe !
		private final Holder holder;
 
		// Holder doit être thread-safe, même s'il est "donné".
		public HolderThread(Holder holder) {
			this.holder = holder;
		}
 
		@Override
		public void run() {
			holder.assertSanity();
		}
	}
 
	private static class HolderThread2 implements Runnable {
 
		// Le final importe !
		private final SynchronousQueue<Holder> rdv = new SynchronousQueue<Holder>();
 
		// Holder n'a pas besoin d'être thread-safe s'il est "donné".
		public HolderThread2(Holder holder) {
			rdv.offer(holder);
		}
 
		@Override
		public void run() {
			try {
				Holder holder = rdv.take();
				holder.assertSanity();
			} catch (InterruptedException e) {
				Thread.currentThread().interrupt();
			}
		}
	}
 
	public void testIt() {
		// Code susceptible de lever une AssertionError.
		new Thread(new HolderThread(new MutableHolder(10))).start();
 
		// Code correct.
		new Thread(new HolderThread(new FinalHolder(10))).start();
		new Thread(new HolderThread(new SynchronizedSafeHolder(10))).start();
		new Thread(new HolderThread(new VolatileSafeHolder(10))).start();
		new Thread(new HolderThread(new AtomicSafeHolder(10))).start();
 
		// Code correct.
		new Thread(new HolderThread2(new MutableHolder(10))).start();
	}
}
N'ayant jamais structuré mon code de cette manière, j'imagine que la quasi-totalité du code multi-threads que j’écris depuis 10 ans est buggé ! En fait, j'ai bien l'impression que le code thread-safe est plus l'exception que la règle, notamment dans le Javadoc de Sun. Si on prend l'exemple suivant :

Code : Sélectionner tout - Visualiser dans une fenêtre à part
1
2
3
4
5
6
7
8
9
10
11
12
class PrimeRun implements Runnable {
 
	long minPrime;
 
	PrimeRun(long minPrime) {
		this.minPrime = minPrime;
	}
 
	public void run() {
		// compute primes larger than minPrime
	}
}
C’est incorrect, non ? Il faudrait que minPrime soit final. En fait, même le code de la classe Thread me semble suspect, puisque le constructeur Thread(Runnable target) stocke target dans une variable non final et l'utilise dans run() sans synchronisation particulière ! Du coup, je ne sais plus quoi penser...