for range loops one time more than it should

[grefab]

Thanks for the library! Just to let you know, while playing with the code I encountered some problems regarding closing when using the channel in a for range loop:

A for range loop iterates once on an empty channel when that channel is closed in another thread.

More general: If you rely on the end iterator to finish a loop it comes one iteration too late.

The reason for this behaviour is that blocking while waiting for data happens in operator*. When the block is released upon closing of the channel (not because data has arrived), an undefined value is retrieved and returned.

This can be avoided when the range loop checks for channel validity like this:

1  for (auto in : incoming) {
2      // Check if we are done
3      if (incoming.empty() && incoming.closed()) {
4          break;
5      }
6      ...
7  }

But that a) looks ugly and b) can still cause errors (data not processed) when closing happens between line 1 and 3 in a different thread.

[andreiavrammsd]

Great feedback, thank you!

I have plans to continue working on the library next year. This issue will remain open until I find a solution. If suggestions come to your mind, they are more than welcome (even as PRs).

[grefab]

I thought about it and the only solutions that seems to work is kinda weird. I assume that the for range loop works as follows:

• see if operator!= is true and if so break
• call operator* and use its result in loop
• call operator++
• repeat

Since only after operator<< it is clear if we have a valid result or are in a closed&&empty state, that must be called in operator!=.

This works:

template <typename Channel>
class BlockingIterator {
public:
    using value_type = typename Channel::value_type;
    explicit BlockingIterator(Channel& ch) : ch{ch} {}
    BlockingIterator<Channel> operator++() const noexcept { return *this; }
    value_type operator*() const noexcept { return value; }
    bool operator!=(BlockingIterator<Channel>) noexcept {
        value << ch;
        return !(ch.closed() && ch.empty());
    }
private:
    Channel& ch;
    value_type value{};
};

But I don't think it is a good solution, because the semantics are mixed up. operator!= cannot be called multiple times without messing with the queue's content. And there is still a race condition here:

value << ch;
return !(ch.closed() && ch.empty());

[andreiavrammsd]

I will give this a thought when I will work on the issue. Thanks!

[andreiavrammsd]

Could you test the fix-iterator-off-by-one-issue branch and see if you can still reproduce the issue?

[grefab]

Hey, thanks! I will test that early January. Happy holidays! :)

[andreiavrammsd]

If there is nothing else to add, I will close the issue soon and merge #9 .

[andreiavrammsd]

Thank you for the feedback!