[ruby-core:48584] Re: [ruby-trunk - Feature #4589][Feedback] add Queue#each() method and include Enumerable

On Tue, Oct 30, 2012 at 08:44:21AM +0900, ko1 (Koichi Sasada) wrote:
> 
> Issue #4589 has been updated by ko1 (Koichi Sasada).
> 
> Status changed from Assigned to Feedback
> Target version set to next minor
> 
> Professional's comments are very welcome.
> I can't determine it should be worth or not.

I don't think it's a good idea.  There are different ways to make a
thread-safe `each`, and they have different behavior.  For example:

  class Queue
    def each(&block)
      @mutex.synchronize {
        @que.each(&block)
      }
    end
  end

compared to:

  class Queue
    def each(&block)
      @que.dup.each(&block)
    end
  end

Both provide each, but the semantics are totally different.  The first
one ensures that no other threads will update the queue, but that blocks
any threads from updating the queue.  The second one doesn't block the
queue, but the values yielded to the block are inconsistent (e.g., a
thread could write to the queue after the internal array has been duped).

I think it's better to do this:

  class Queue
    def synchronize
      @mutex.synchronize { yield }
    end

    def to_a; @que.dup; end
  end

As a user, it's obvious you're getting a copy:

  queue = Queue.new
  queue.to_a.each { |item| ... }

If you want to block other threads from manipulating the queue while
iterating:

  queue = Queue.new
  queue.synchronize do
    queue.to_a.each { |item| ... }
  end

TL;DR: adding `sychronize` and `to_a` would be more powerful and less
invasive than adding `each` (IMO).

-- 
Aaron Patterson
http://tenderlovemaking.com/