Concurrent Javadoc

Writing correct concurrent code is hard and what I'm finding equally challenging is documenting concurrent behavior in code in prose (javadoc to be specific).

Pre Java 1.5 all we had where java.lang.Thread, the Runnable interface, the synchronized keyword, and volatile variables. Because synchronized was the only real mechanism for concurrency control if you wanted to reason about the thread safety of a class all you had to do was look for the synchronized keyword. Some of you may be asking "but what about volatile?" volatile then was nowhere near as powerful as it is now. Back then the only thing volatile really meant was do not cache. Nowadays volatile not only means do not cache it also affects the visibility of other non volatile variables and it also imposes ordering constraints. Basically synchronized and volatile no longer act in isolation. All activities inside the JVM is now governed by a well defined memory model (JMM). It's no longer enough to look for the synchronized keyword you have to keep the memory model in mind at all times which is damn hard to document. Therefore I'm seeking feedback.

Imagine you have been asked to maintain a body of code with which you have had no previous experience and the only thing you know is it's targeted at Java 1.5 and higher (new JMM). You come across this (partial) class definition:

1    /**
2     * Ties {@link BlastMaster}, {@link BlastManager}, and {@link BlastAgent} objects together around a blast. It is the
3     * highest layer of abstraction available on a blast. It provides the hooks for the lifecycle management of a blast and
4     * is the communication channel between BlastAgent(s) and BlastManager(s) and BlastMaster.
5     * <p>
6     * The lifecycle of a context goes something like this. It is created by BlastMaster, injected into the outbound queue
7     * by BlastMaster, scheduled by BlastManager, delivered by BlastAgent(s), descheduled by BlastManager, rescheduled by
8     * BlastManager, delivered by BlastAgent(s) ....
9     *
10    * The scheduling, delivering, and descheduling steps repeat until a) all the recipients have been sent the blast or b)
11    * the duration of the blast has expired.
12    * </p>
13    *
14    * @author HashiDiKo
15    */
16   public final class BlastContext extends Constants
17   {
18       /**
19        * Keeps track of the number of domains yet to be delivered. When this number reaches zero it means the current
20        * iteration of trying to deliver the blast has ended.
21        *
22        * @see BlastAgent#decrementRemaining()
23        * @see #fire(ContextEvent.Event)
24        */
25       final AtomicInteger remaining = new AtomicInteger();
26   
27       /**
28        * Provides a mechanism for a {@link BlastManager} to be notified when the {@link BlastAgent}s executing the context
29        * stop executing it.
30        * <p> This object does a great deal in terms of establishing <em>happens-before</em> edges between a blast manager
31        * and its agents and between blast agents.</p>
32        */
33       final Set<Thread> agents = Collections.synchronizedSet( new HashSet<Thread>( MAX_BLAST_AGENTS + 1) );
34   
35       /**
36        * The message.
37        *
38        * <h4>Visibility</h4>
39        *
40        * <p>The blast manager thread is the only thread that writes this field while blast agents are the only threads
41        * to read this field. This field is <i>correctly synchronized</i> because the blast master writing this field
42        * <b>always</b> <i>happens-before</i> the blast agents reading it. This <i>happens-before</i> relationship is
43        * established in the code starting with {@link #init()} and its call to {@link #doInit()}. {@link #doInit()} is
44        * the only place in the code where this field is written. You simply need to follow the flows in
45        * {@link BlastManager} starting from where {@link #init()} is called to see synchronization the points that
46        * establishes the <i>happens-before</i> relationship between the blast manager and its blast agents in regard to
47        * this field.
48        * </p>
49        */
50       Message message;
51   
52       /**
53        * Indicates that this context is context to an old blast. In other words the system was restarted and the blast
54        * was loaded from the database.
55        *
56        * @see #init()
57        * @see #BlastContext(boolean, EmailBlast, BlastMaster)
58        */
59       private final boolean old;
60   
61       /**
62        * The home directory of the blast.
63        */
64       private final File home;
65   
66       /**
67        * The duration of the blast in milliseconds.
68        */
69       private final long durationMillis;
70   
71       /**
72        * Used to implement our backoff algorithm.
73        *
74        * <h4>Visibility</h4>
75        *
76        * <p>The field is only ever read/written by blast agents thus the visibility of this field is limited to
77        * blast agents in general and specifically the {@link #fire(ContextEvent.Event) fire} method. Even though
78        * {@link #fire(ContextEvent.Event) fire } is not <tt>synchronized</tt> and this field is non volatile it is
79        * <em>correctly synchronized</em>. There are 2 things at work that make this so. Firstly, there is never
80        * concurrent access to this field. At most 1 blast agent thread will write/read this field for a delivery cycle.
81        * This is guranteed by {@link BlastAgent#decrementRemaining()} because a) how its implemented and b)it is the only
82        * place where a blast agent get accesses this field. Finally, the fact that blast agents call
83        * {@link #register(Thread)} before they execute a blast and {@link #unregister(Thread)} when they are done
84        * executing a blast (both contained <tt>synchronized</tt> blocks using the same monitor) gurantees that the update
85        * of this field will be visible to any subsequent reads by different blast agents.
86        * </p>
87        * @see #fire(ContextEvent.Event)
88        */
89       private int magnitude;
90   
91       /**
92        * Used to implement our backoff algorithm.
93        *
94        * <h4>Visibility</h4>
95        *
96        * <p>The field is only ever read/written by blast agents thus the visibility of this field is limited to
97        * blast agents in general and specifically the {@link #fire(ContextEvent.Event) fire} method. Even though
98        * {@link #fire(ContextEvent.Event) fire } is not <tt>synchronized</tt> and this field is non volatile it is
99        * <em>correctly synchronized</em>. There are 2 things at work that make this so. Firstly, there is never
100       * concurrent access to this field. At most 1 blast agent thread will write/read this field for a delivery cycle.
101       * This is guranteed by {@link BlastAgent#decrementRemaining()} because a) how its implemented and b)it is the only
102       * place where a blast agent get accesses this field. Finally, the fact that blast agents call
103       * {@link #register(Thread)} before they execute a blast and {@link #unregister(Thread)} when they are done
104       * executing a blast (both contained <tt>synchronized</tt> blocks using the same monitor) gurantees that the update
105       * of this field will be visible to any subsequent reads by different blast agents.
106       * </p>
107       * @see #fire(ContextEvent.Event)
108       */
109      private boolean hrs;
110  
111      /**
112       * Stores the backoff timeout (the end result of the backoff algorithm computation).
113       *
114       * <h4>Visibility</h4>
115       *
116       * <p>The visibility requirements for this field is kind of funky. It's is a read/write field for
117       * {@link BlastManager}s and write only field for {@link BlastAgent}s. So the core visibilty issue is writes by
118       * blast agents being visible to blast manager. The <em>happens-before</em> edge that makes this possible is
119       * established by the monitor acquisition of {@link #agents} by the blast agent thread that writes this field and
120       * the subsequent acquisition of the same monitor by the blast manager.
121       * </p>
122       *
123       * @see #fire(ContextEvent.Event)
124       */
125      private long backoffTimeout;
126  
127      /**
128       * Indicates if {@link #doInit()} has been called.
129       * This field is only ever read/written by the blast manager thread for this context thus it is correctly
130       * synchronized.
131       */
132      private boolean didInit;
133  
134      /**
135       * Temporarily store the short domain blacklist expiration.
136       * <p>
137       * When this value is greater than zero it indicates to the blast manager thread that even though {@link #domainQ}
138       * is empty this context has undelivered recipients.
139       * </p>
140       * <p>This field is <em>correctly synchronized</em> because it is only ever read/written by the blast manager
141       * thread.</p>
142       *
143       * @see #getDomains()
144       */
145      private long shortestDomainTimeout;
146  
147      /**
148       * Gate to {@link #manager}. It establishes a <em>happens-before</em> edge with {@link #manager}.
149       */
150      private final CountDownLatch managerLatch = new CountDownLatch( 1 );
151  }
152  

So. Does the javadoc comments say enough about how non volatile class members manage to be correctly synchronized?