Thanks for your in-depth analysis. In conclusion, the 11.5 listing is broken specifically with the reification of the seq function. The problem is that the seq function allows the array reference to escape in an unsafe manner. The issue is concurrency as well as visibility. As you suggest, the only fix is to safely copy (via the locking function, i.e. synchronized block) the contents of the array into a data structure that would be passed back to the calling code.
By the way, I still think you need a lock in the count function in the case where the caller tries to invoke the count function on a partially constructed object. You may be able to reason otherwise, but it is simply confusing to do so. Just access shared mutables in a safe manner and you will be OK. On Jul 29, 3:38 pm, Ken Wesson <kwess...@gmail.com> wrote: > On Fri, Jul 29, 2011 at 3:19 PM, Julien <julien.c.chast...@gmail.com> wrote: > > This listing is an attempt to make the function safe for concurrent > > modification. My claim is that count and seq should also be locking > > around "a" for exactly same reason as aget and aset. In particular, > > locking is not only ensuring mutual exclusion but also *visibility*. > > The danger is that count and seq may be accessing stale values of "a". > > In the case of "count" that danger doesn't exist; Java arrays are of > fixed size from creation, so the "count" value cannot change. > > The case of "seq" is trickier: the array can indeed change contents > midway through iteration. But there's no simple fix. One could write > something like > > (let [cnt (dec (count a)) > step (fn step [i] > (lazy-seq > (if (> i cnt) > (do (monitorexit a) nil) > (cons x (step (inc i))))))] > (monitorenter a) > (step 0)) > > to hold a lock all the way through seq traversal, but if the seq is > only partially traversed (either deliberately, or because of a thrown > exception) the lock will never get released. > > More complicatedly, you could create a CloseableSeq defprotocol that > extends ISeq and Closeable and create closeable seqs (line-seq could > benefit from this as well); for the above the close operation would be > (monitorexit a) (and for line-seq (.close rdr)). Closeable seqs could > then be created in (with-open ...) and handled in an exception-safe > manner -- so long as you made sure to consume or doall the seq before > the end of the with-open scope. In the array seq case, the seq would > still seem to work after the scope exited, but it would show an array > in a possible state of flux again instead of a fixed state. > > Given all this complication, easiest might be > > (seq [this] > (locking a > (seq (into [] a)))) > > which has the downside of copying the array into a vector but the > upside of being thread- and exception-safe; the seq is backed by a > fixed snapshot of the array. And can be traversed lazily and at > leisure without holding onto a lock the whole time; the lock's held > only during the copying. If you're worried about seeing inconsistent > cell values during a traversal of a short array this is what you ought > to use. > > But you should probably prefer to avoid arrays for the most part. :) > > Note: you will still potentially see *stale* values; the array can > change after the snapshot is made. To avoid that you'd always have to > lock during the operation where you want to be seeing the most up to > date values. But you wouldn't see *inconsistent* values. If the array > contains all 1s, and then someone else locks it, changes each cell to > 2, and unlocks, and you are traversing the seq given above, you might > get back (1 1 1 1 2 2 2 ...) which violates the intended invariant of > all-the-same-number. With the snapshot you'll possibly see all-1s even > after it's been updated to all-2s but you will never see a mixture. > With locking the array the whole time you're working with the seq, > you'll see all-2s if the array was ever updated to all-2s and > otherwise the thing that will eventually update it to all-2s will > block until you're done with the seq. > > > See Java Concurrency in Practice, Section 3.1 for more of a > > discussion. My reasoning is based on the assumption that locking is > > simply an abstraction for the Java synchronized keyword. That > > assumption may or may not be correct. > > It is correct, though Clojure exposes the lower-level monitorenter and > monitorexit; both Java synchronized and Clojure locking boil down to > (the bytecode emitted by) > > (try > (monitorenter foo) > ... > (finally (monitorexit foo))) > > so as to ensure the lock is released even if an exception is thrown > out of the critical section. Java just doesn't let you get at the > separate lock and unlock operations for normal objects (but there are > the java.util.concurrent lock object classes). > > -- > Protege: What is this seething mass of parentheses?! > Master: Your father's Lisp REPL. This is the language of a true > hacker. Not as clumsy or random as C++; a language for a more > civilized age. -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en
