Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object became nil #2925

Closed
dzjuck opened this issue May 10, 2015 · 3 comments
Closed

Object became nil #2925

dzjuck opened this issue May 10, 2015 · 3 comments

Comments

@dzjuck
Copy link

dzjuck commented May 10, 2015

I'm trying to add JRuby support to Hamster gem, and one spec doesn't work with JRuby:

Steps to reproduce:

git clone git@github.com:hamstergem/hamster.git
cd hamster
bundle

rspec ./spec/lib/hamster/list/partition_spec.rb:39

Spec code

    it "calls the passed block only once for each item, even with multiple threads" do
      mutex = Mutex.new
      yielded = [] # record all the numbers yielded to the block, to make sure each is yielded only once
      list = Hamster.iterate(0) do |n|
        sleep(rand / 500) # give another thread a chance to get in
        mutex.synchronize { yielded << n }
        sleep(rand / 500)
        n + 1
      end
      left, right = list.partition(&:odd?)

      10.times.collect do |i|
        Thread.new do
          # half of the threads will consume the "left" lazy list, while half consume
          # the "right" lazy list
          # make sure that only one thread will run the above "iterate" block at a
          # time, regardless
          if i % 2 == 0
            left.take(100).sum.should == 10000
          else
            right.take(100).sum.should == 9900
          end
        end
      end.each(&:join)
    end

error happens in one of two strings:

            left.take(100).sum.should == 10000
            right.take(100).sum.should == 9900

Code where happens error:

  class Partitioned < Realizable
    def initialize(partitioner, buffer, mutex)
      super()
      @partitioner, @buffer, @mutex = partitioner, buffer, mutex
    end

    def realize
      @mutex.synchronize do
        return if @head != Undefined # another thread got ahead of us
        while true
          if !@buffer.empty?
            @head = @buffer.shift
            @tail = Partitioned.new(@partitioner, @buffer, @mutex)
            # don't hold onto references
            # tail will keep references alive until end of list is reached
            @partitioner, @buffer, @mutex = nil, nil, nil
            return
          elsif @partitioner.done?
            @head, @size, @tail = nil, 0, self
            @partitioner, @buffer, @mutex = nil, nil, nil # allow them to be GC'd
            return
          else
            @partitioner.next_item
          end
        end
      end
    end
  end

Error happens on string:

      @mutex.synchronize do
     Failure/Error: left.take(100).sum.should == 10000
     NoMethodError:
       undefined method `synchronize' for nil:NilClass

A couple of words about what happens here:

  • spec creates lazy list, which is iterated over by tail recursion in 10 threads.
  • @mutex is created in parent lazy list and is used in all descendant lists
  • Partition#realize is called exactly once per object

Error doesn't happen in MRI and Rubinius.

It is possible to count errors, if we add 'rescue' to the end of Partitioned#realize:

    rescue NoMethodError => e
      puts "!!! ERROR NO_METHOD_ERROR !!! @mutex: #{@mutex.object_id}"

On my laptop error count is 4 in average.

If in Partitioned#realize

          if !@buffer.empty?
            @head = @buffer.shift
            @tail = Partitioned.new(@partitioner, @buffer, @mutex)
            # don't hold onto references
            # tail will keep references alive until end of list is reached
            @partitioner, @buffer, @mutex = nil, nil, nil
            return

we remove @mutex = nil

          if !@buffer.empty?
            @head = @buffer.shift
            @tail = Partitioned.new(@partitioner, @buffer, @mutex)
            # don't hold onto references
            # tail will keep references alive until end of list is reached
            @partitioner, @buffer = nil, nil
            return

error stops to happen.

Possibly it's a bug of garbage collector.
I don't understand what exactly triggers this error. I tried several times to make independent example to reproduce this bug - without success. If anyone has any idea how to make it - i will implement it gladly.

Reproduced in both JRuby versions - 1.7.19 and 9000.

Thank you.

@alexdowad
Copy link
Contributor

I believe this is a threading bug in the hamster gem, which was uncovered when running the specs on JRuby, simply because JRuby allows greater concurrency.

@kares
Copy link
Member

kares commented May 14, 2015

+1 ... esp. since from a quick look it's obvious that the very same code does set @mutex = nil so it's very likely a bug in the hamster code itself (not counting for all concurrent cases). also it's not really needed to reset the @mutex (or any others - would keep it immutable) it does not help the GC much if at all.

@kares kares added this to the Invalid or Duplicate milestone May 14, 2015
@alexdowad
Copy link
Contributor

@kares, setting @mutex to nil definitely doesn't help the GC if you drop all references to the lazy list itself. It definitely does help the GC if you retain references to the list. If you have many thousands or millions of such lists "live" in memory at the same time, it helps the GC a great deal. It just needs to be done in a thread-safe way -- making sure that no thread will attempt to use it after it has been dropped.

@dzjuck dzjuck closed this as completed May 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants