Completed
Details
Details
Assignee
Unassigned
UnassignedReporter
Alex Miller
Alex MillerLabels
Approval
Ok
Patch
Code
Priority
Affects versions
Fix versions
Created January 11, 2015 at 5:34 AM
Updated January 18, 2015 at 5:36 PM
Resolved January 18, 2015 at 5:36 PM
As of 1.7.0-alpha5, we are seeing SeqIterator return iterated results that do reflect the values of the underlying seq, in particular acting as if the seq contains a nil value when it does not. This problem is intermittent but has at times caused clojure master to fail in compilation (which is why this is marked as a blocker).
Two recent changes during 1.7 have created and exposed this problem:
1) This commit https://github.com/clojure/clojure/commit/43cc1854508d655e58e377f84836ba128971f90c changed the SeqIterator implementation to be lazier and to use "this" as a sentinel object in SeqIterator. (1.7.0-alpha2)
2) CLJ-1546 changed the implementation of vec such that PersistentHashMap and PersistentHashSet are now converted using iterator() rather than seq(). PHS/PHM use SeqIterator for their Iterator implementation. (1.7.0-alpha5)
Because of #2, we are now stressing #1 much more than before. In particular, things like defining defrecords rely heavily on vec (and set) of PHS and PHM.
Example stack trace: https://gist.github.com/puredanger/f56e3253f0668a515ec5 (seen compiling Clojure itself)
Cause: Setting
seq==this;
in the constructor of SeqIterator is allowing unsafe publication of the partially constructed "this" object, which can cause subtle problems in thehasNext()
implementation. In particular, it seems that after inlining, on the first call, theseq==this
condition when comparing the cached partially constructed instance inseq
and the fully constructed version inthis
will return false, even though these have the same object identity. This causes the wrong path to be executed in hasNext().Approach: Do not use
this
as a sentinel value.Patch: 0001-CLJ-1636-don-t-use-this-as-a-sentinel-in-SeqIterator.patch
Screened by: Alex Miller