Getting Default-Initialised Value after Channel is Closed / use std::optional for Popping?

[stellarpower]

Hi,

I think I am repeatedly hitting an issue like the following:

• producer has pushed its last value
• consumer successfully pops this.
• consumer now blocks on popping the next value
• producer closes the channel
• consumer has an empty default-initialised T, which it treats like a real request.

Glossing over the source here I think there may be a potential issue with checking if the channel is closed before acquiring the mutex - it's possible that the channel is open when we check, then the producer closes it, then we acquire the lock. Without having investigated it yet in more detail, I think I'd want to check if the channel is closed again after we acquire the lock. but before I spend any time trying to pin it down and verify if it's a bug, wondering if others are seeing a situation like this in their use of the library, or if the fault is just on my end.

However, as an aside to this, whilst the stream operators are a nice API, I feel that even aside from this specific scenario, it's then hard to tell if there really is a value or not. Let's say the channel is closed in the first check, and we just return. I now have a (presumably) default-initialised value on the stack, and I don't actually know if the channel read anything or not. By streaming into something, we always have a something, whether or not there was a successful pop, so we need a manual method to check the validity of that something.

Personally, I almost always like to use std::optional for this sort of thing. I the existing API works for people, then can leave it as it is, but I think it might be easier to have a function optional<T> pop(), where the return value is nullopt if the channel was closed or empty, or contains a value if the read was successful. Or otherwise we could throw an exception, for those that want to use them, but optional seems lighter to me. So, please do correct me if I'm missing something here, but I think if I do:

int latestValue;
channel >> latestValue;

then I no have no way of knowing if the read was successful, as operator>> just no-ops if there was an issue, so out is left as it was, and I rely on manually checking that integer to see if it's valid. Potentially meaning that e.g. the value 0 would be out of bounds for values to push, I have to have some sort of empty value I can check for. I can ask the channel if it is closed or empty after the read - but then we have a race condition between the point the pop was performed and the point I perform the check.

So, I know boost's lockfree queues return a boolean to indicate success and take by reference in a similar manner, which would achieve the same, but in my own code I always stick to optional (and I'm actually wrapping Channel to do that right now) so that I have a clear idea of if a read was successful or not, and then I don't have to think about it, and I don't get any off-by-one issues downstream. Plus the compiler then enforces me to check the value, whilst a boolean return status can easily be left off and forgotten about.

Thoughts? Thanks!

[andreiavrammsd]

Hi,

Your issue is most likely valid. The initial implementation of the channel did not have the closable feature, so overloading those operators did not have an issue. Sometimes I don't even know if this feature is absolutely needed.

This channel implementation is purely for learning purposes. Coming from Go, I thought this topic would be a good starting point for a pet project. I didn't expect anyone to really use it. So I don't have any real perspectives over its future. I don't even know why somebody would use this implementation over the other existing ones, which are battle-tested and have many (or active) maintainers.

For sure std::optional is a more strong-typed alternative over a boolean. I would need to implement it to maintain the C++11 compatibility.

From time to time I do some fixes for issues that I know, but I don't have a full-time commitment to this project. This is why I keep the major version to 0. My honest recommendation is to search for a more stable implementation or to open small pull requests if you want specific changes or fixes. Some hard requirements: C++11 compatibility, the << >> operators to be still used, and the implementation to be as simple as possible.

Thanks for using the channel! Real usage is the best test.