Feature #5958: ThreadGroup#join

• Status: Rejected
• Assignee: ko1 (Koichi Sasada)
• Target version: -

[Glass_saga (Masaki Matsushita)]

I propose the method ThreadGroup#join.
Calling thread waits all threads in receiving threadgroup.

thgrp = ThreadGroup.new
thgrp.add(Thread.new{ sleep 1 })
thgrp.join #=> #<ThreadGroup:0x007ff257d6d098>

Time limit to run the threads in the threadgroup can be specified.
If the time limit expires, nil will be returned.

thgrp = ThreadGroup.new
thgrp.add(Thread.new{})
thgrp.add(Thread.new{ sleep })
thgrp.join(0.1) #=> nil

I think it is useful to wait grouped threads together.

[kosaki (Motohiro KOSAKI)]

• Status changed from Open to Rejected

You didn't explained any usecases. So, It'd cause automatically reject.

[regularfry (Alex Young)]

On 01/02/12 20:33, Motohiro KOSAKI wrote:

Issue #5958 has been updated by Motohiro KOSAKI.

Status changed from Open to Rejected

You didn't explained any usecases. So, It'd cause automatically reject.

I can't off the top of my head think of a script where I've done:

threads = things.map { |thing| Thread.new{ process(thing) } }

where I haven't followed it with:

threads.each { |thread| thread.join }

It's always confused me that ThreadGroup doesn't have a #join method, to
the extent that without it I don't know what a ThreadGroup is supposed
to be for. I've certainly never used one.

--
Alex

[kosaki (Motohiro KOSAKI)]

• Status changed from Rejected to Open

[Glass_saga (Masaki Matsushita)]

Without ThreadGroup#join, we have to get list of threads and join each thread in the ThreadGroup.

thgrp = ThreadGroup.new

3.times do |i|
  thgrp.add(Thread.new{ sleep i+1 })
end

thgrp.list.each(&:join)

Moreover, it is confusing to set limit on joining in the situation.

class ThreadGroup
  def join(delay=nil)
    if delay
      limit = Time.now + delay
      if self.list.all?{|th| th.join(limit - Time.now.to_f) }
        return self
      else
        return nil
      end
    else
      self.list.each(&:join)
      return self
    end
  end
end

ThreadGroup#join enables to join threads as a unit of ThreadGroup.
I think ThreadGroup will be more useful if it provides methods to manipulate its own threads like this.

[ko1 (Koichi Sasada)]

What should happen when at least one thread causes an exception?

[Glass_saga (Masaki Matsushita)]

Koichi Sasada wrote:

What should happen when at least one thread causes an exception?

Under the patch, joining thread also causes the exception if the thread is being joined.
Otherwise, nothing will happen on joining thread.
It behaves the same as thgrp.list.each(&:join).

I think ThreadGroup#join may be more useful if it catches exceptions from all its threads and handles them like the following.

thgrp.join do |th| # each thread which causes an exception
  # handle an exception
end

[ko1 (Koichi Sasada)]

(2012/02/26 17:32), Masaki Matsushita wrote:

I think ThreadGroup#join may be more useful if it catches exceptions from all its threads and handles them like the following.

thgrp.join do |th| # each thread which causes an exception
  # handle an exception
end

This behavior is very confusing for me.
I know it is difficult design problem.

This is my 1 min considered version:

def join
  @threads.map{|th|
    begin
      result = th.join
    rescue Exception => e
      result = e
    end
    [th, e]
  } # or return Hash (th => result)
end

--
// SASADA Koichi at atdot dot net

[regularfry (Alex Young)]

On 26/02/2012 08:32, Masaki Matsushita wrote:

Issue #5958 has been updated by Masaki Matsushita.

Koichi Sasada wrote:

What should happen when at least one thread causes an exception?

Under the patch, joining thread also causes the exception if the thread is being joined.
Otherwise, nothing will happen on joining thread.
It behaves the same as thgrp.list.each(&:join).

I think ThreadGroup#join may be more useful if it catches exceptions from all its threads and handles them like the following.

thgrp.join do |th| # each thread which causes an exception
   # handle an exception
end

It'd be much less surprising to go through the normal rescue mechanism
if possible. How about something like this:

module ThreadGroupExceptions
  attr_accessor :next_in_group

  def each
    current = self
    while current
      yield current
      current = current.next_in_group
    end
  end
end

class ThreadGroup
  def join
    group_exc = nil
    list.each do |thread|
      begin
        thread.join
      rescue => exc
        exc.extend ThreadGroupExceptions
        group_exc, exc.next_in_group = exc, group_exc
      end
    end
    raise grp_exc if grp_exc
  end
end

It's slightly messy in that if you're not expecting there to be a
ThreadGroup involved then you won't be aware of the mixin, but if you
are, then everything's available as a linked list (which an Enumerable
could be layered on, if needed), and you can just rescue ThreadGroupExceptions in that case.

--
Alex

[drbrain (Eric Hodel)]

As another design restriction, the thread list should ensure that threads started in the ThreadGroup appear at the end of the list and not randomly to ensure all the threads are joined. Otherwise, joining a ThreadGroup may leave threads running.

Enclosing the ThreadGroup will help as existing threads cannot be added or removed, but threads in the group may still spawn new threads.

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned
• Assignee set to ko1 (Koichi Sasada)

Sasada-san, what do you think?

--
Yusuke Endoh [e-mail]

[ko1 (Koichi Sasada)]

• Status changed from Assigned to Rejected

Now, I reject this ticket.

I think your proposal is very useful.
To introduce thread set manipulation is needed.

But I feel it is not mature.
Please re-design ThreadGroup and make another tickets.

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)

[nobu (Nobuyoshi Nakada)]

• Related to Feature #15954: 簡単にマルチスレッドを一度に合流させる added