Skip to content

Conversation

@acco
Copy link
Contributor

@acco acco commented Jun 30, 2025

Problem

Addresses #315

The GenStage.Buffer module was returning permanent events in LIFO instead of FIFO order when multiple permanent events were stored at the same wheel position.

Reproduction

alias GenStage.Buffer

# Create a buffer and store some temporary events
buffer = Buffer.new(10)
{buffer, _excess, _perms} = Buffer.store_temporary(buffer, [:temp1, :temp2], :first)

# Store multiple permanent events at the same position
{:ok, buffer} = Buffer.store_permanent_unless_empty(buffer, :perm_first)
{:ok, buffer} = Buffer.store_permanent_unless_empty(buffer, :perm_second)
{:ok, buffer} = Buffer.store_permanent_unless_empty(buffer, :perm_third)

# Take events - notice the permanent events are in wrong order
{:ok, _buffer, _remaining, temps, perms} = Buffer.take_count_or_until_permanent(buffer, 5)

IO.inspect(temps)  # [:temp1, :temp2] ✓ Correct FIFO order
IO.inspect(perms)  # [:perm_third, :perm_second, :perm_first] ❌ Wrong! Should be FIFO

Before fix: [:perm_third, :perm_second, :perm_first] (LIFO - wrong)
After fix: [:perm_first, :perm_second, :perm_third] (FIFO - correct)

Root Cause

In pop_and_increment_wheel/1, permanent events were stored using [new_perm | existing_list], but when retrieved the list wasn't reversed to restore FIFO order.

Solution

Added :lists.reverse(perms) in pop_and_increment_wheel/1 before returning permanent events, ensuring they're returned in the same order they were stored.


alias GenStage.Buffer

describe "estimate_size/1" do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to remove any extraneous tests not related to this commit!

Buffer.take_count_or_until_permanent(buffer, 5)

assert temps == [:temp1, :temp2]
assert perms == [:perm3, :perm4, :perm5]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was failing before fix

assert perms == [:perm3, :perm4, :perm5]
end

test "interleaves temporary and permanent events correctly across multiple wheel positions" do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was also failing before fix

@acco acco force-pushed the acco/fix-order-of-queued-sync_info-messages branch from 556c6b7 to d2e8611 Compare June 30, 2025 03:27
@josevalim josevalim merged commit 60f4d50 into elixir-lang:main Jun 30, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants