Hi David,
On Sun, 7 Feb 2021, David T. Lewis wrote:
This looks good to me, and it fixes the failing test.
Note that none of the historical versions of SharedQueue>>nextOrNilSuchThat: pass the test, but the issue became visible after CollectionsTests-eem.342 last September when the existing test for SharedQueue2 was applied to SharedQueue as well.
It would be good if Levente or Nicolas can review this, but +1 from me.
The fix looks right in the sense that it does what the test case requires. But it may affect the way events are processed. The only sender of this method is EventSensor which may rely on the current behavior. So, I would like someone who understands how EventSensor works confirm that the change is okay before pushing it to the Trunk.
Levente
Thanks Jaromir for putting this in the inbox :-)
Dave
On Sun, Feb 07, 2021 at 08:42:11AM +0000, commits@source.squeak.org wrote:
A new version of Collections was added to project The Inbox: http://source.squeak.org/inbox/Collections-jar.924.mcz
==================== Summary ====================
Name: Collections-jar.924 Author: jar Time: 7 February 2021, 9:42:07.649004 am UUID: 073b26bc-8e5e-ba41-9aac-9d424bbe6848 Ancestors: Collections-dtl.923
Fixes SharedQueue >> nextOrNilSuchThat bug to pass the existing test
=============== Diff against Collections-dtl.923 ===============
Item was changed: ----- Method: SharedQueue>>nextOrNilSuchThat: (in category 'accessing') ----- nextOrNilSuchThat: aBlock "Answer the next object that satisfies aBlock, skipping any intermediate objects. If no object has been sent, answer <nil> and leave me intact. NOTA BENE: aBlock MUST NOT contain a non-local return (^)."
^accessProtect critical: [ | value readPos | value := nil. readPos := readPosition. [ readPos < writePosition and: [ value isNil ] ] whileTrue: [ value := contentsArray at: readPos. readPos := readPos + 1. (aBlock value: value) ifFalse: [ value := nil ] ifTrue: [ readSynch waitIfLocked: [ ^nil ]. "We found the value, but someone else booked it."
readPos-1 to: readPosition+1 by: -1 do: [ :j | contentsArray at: j put: (contentsArray at: j-1) ].contentsArray at: readPosition put: nil.readPosition := readPosition+1 ] ].
readPosition to: readPos - 1 do: [ :j | contentsArray at: j put: nil ].readPosition := readPos ] ].