Including a separate ruby file with a proto definition that has a recursive field results in RuntimeError: can't modify frozen SystemStackError

[bufdev]

Maybe I'm missing something.

janus.proto imports jet.proto.

If I have two generated protos, jet.rb and janus.rb, in different packages:

After importing jet.rb, if I then import jet.rb, I will get:

2.2.2 :001 > require 'janus'
RuntimeError: can't modify frozen SystemStackError
    from /Users/peter/.rvm/gems/ruby-2.2.2/gems/janus-0.1.0/_ext/ruby/lib/janus/janus.rb:6:in `finalize_to_pool'
    from /Users/peter/.rvm/gems/ruby-2.2.2/gems/janus-0.1.0/_ext/ruby/lib/janus/janus.rb:6:in `build'
    from /Users/peter/.rvm/gems/ruby-2.2.2/gems/janus-0.1.0/_ext/ruby/lib/janus/janus.rb:6:in `<top (required)>'

This actually seems correct, because what I think is happening is there is a global pool that is then finalized.

1. If I do this:

JANUS_POOL = Google::Protobuf::DescriptorPool.new
JANUS_POOL.build do

And then reference `JANUS_POOL instead:

  Project = JANUS_POOL.lookup("janus.Project").msgclass

Which is not generated code obviously...this doesn't work because the pools don't know about each other (is there a way to merge pools?)

This seems to be a big problem for anyone who has protos that import each other. Am I missing something?

[ngauthier]

Have you listed jet as a gem dependency of janus?

[bufdev]

Yes

[bufdev]

I actually tried both:

1. Having jet be a separate gem, and then including it, both with fully qualified paths and not
2. Compiling jet into janus but in a separate file.

It's a core global state issue

[cfallin]

A few things:

• This case should "just work". I'll look into it.
• There is a single global pool (Google::Protobuf::DescriptorPool.generated_pool), and this is intentional: it's how a proto defined in one place finds a proto in another.
• Finalization is per-message, not per-pool. The build block finalizes everything that was defined within it but the intent, certainly, is that many things can be finalized at different times.

I'll figure out what's going on.

[bufdev]

I'm 99% sure I'm just messing it up, but happy to give concrete reproduce
steps

On Tuesday, May 26, 2015, Chris Fallin notifications@github.com wrote:

A few things:

• This case should "just work". I'll look into it.
• There is a single global pool (
  Google::Protobuf::DescriptorPool.generated_pool), and this is
  intentional: it's how a proto defined in one place finds a proto in another.
• Finalization is per-message, not per-pool. The build block finalizes
  everything that was defined within it but the intent, certainly, is that
  many things can be finalized at different times.

I'll figure out what's going on.

—
Reply to this email directly or view it on GitHub
#425 (comment).

[cfallin]

Cool, if you could share your specific build steps and toplevel Ruby that's require-ing the generated code, that would help. I'll see if I can reproduce it in the meantime. Thanks!

[cfallin]

I just verified I can require one generated .rb file that requires another generated .rb file, with dependent message types.

Something more subtle must be going on. Are you able to share the contents of your .proto files? Is SystemStackError one of your types? Do you by any chance have a duplicate name, or are trying to modify the definition after the fact somehow?

[bufdev]

let me get something together and I'll send it on, it's probably me :)

On Tuesday, May 26, 2015, Chris Fallin notifications@github.com wrote:

I just verified I can require one generated .rb file that requires
another generated .rb file, with dependent message types.

Something more subtle must be going on. Are you able to share the contents
of your .proto files? Is SystemStackError one of your types? Do you by
any chance have a duplicate name, or are trying to modify the definition
after the fact somehow?

—
Reply to this email directly or view it on GitHub
#425 (comment).

[bufdev]

OK @cfallin I figured out what the issue is, and I put this together to reproduce: https://github.com/peter-edge/ruby-protobuf-bug

If you have another file with a message that has a recursive field, it will error.

Just clone that and type make to see it. Hope this helps.

[cfallin]

Thanks. @haberman will be handling the Ruby work from now on -- Josh, could you take a look?

[haberman]

Thanks for the report! I was able to reproduce this error and reduce it to the following test case:

#!/usr/bin/ruby

require 'google/protobuf'

Google::Protobuf::DescriptorPool.generated_pool.build do
  add_message "A" do
    optional :a, :message, 1, "A"
  end
end

Google::Protobuf::DescriptorPool.generated_pool.build do
end

Running this program yields:

test.rb:11: stack level too deep (SystemStackError)

This is a stack overflow in upb. C backtrace is:

#0  0x00007ffff5521f65 in upb_strtable_lookup2 (t=t@entry=0x7fffffffd790, key=key@entry=0xb5ece0 "A", len=1, v=v@entry=0x7fffff7ff040) at upb.c:4052
#1  0x00007ffff552551a in upb_strtable_lookup (v=0x7fffff7ff040, key=0xb5ece0 "A", t=0x7fffffffd790) at upb.h:804
#2  upb_resolve_dfs (def=0xb5e7c0, addtab=addtab@entry=0x7fffffffd790, new_owner=new_owner@entry=0xb52800, seen=seen@entry=0x7fffffffd7b0, s=s@entry=0x7fffffffd850)
    at upb.c:3483
#3  0x00007ffff552558a in upb_resolve_dfs (def=0xb5e7c0, addtab=addtab@entry=0x7fffffffd790, new_owner=new_owner@entry=0xb52800, seen=seen@entry=0x7fffffffd7b0, 
    s=s@entry=0x7fffffffd850) at upb.c:3497
#4  0x00007ffff552558a in upb_resolve_dfs (def=0xb5e7c0, addtab=addtab@entry=0x7fffffffd790, new_owner=new_owner@entry=0xb52800, seen=seen@entry=0x7fffffffd7b0, 
    s=s@entry=0x7fffffffd850) at upb.c:3497
#5  0x00007ffff552558a in upb_resolve_dfs (def=0xb5e7c0, addtab=addtab@entry=0x7fffffffd790, new_owner=new_owner@entry=0xb52800, seen=seen@entry=0x7fffffffd7b0, 
    s=s@entry=0x7fffffffd850) at upb.c:3497
#6  0x00007ffff552558a in upb_resolve_dfs (def=0xb5e7c0, addtab=addtab@entry=0x7fffffffd790, new_owner=new_owner@entry=0xb52800, seen=seen@entry=0x7fffffffd7b0, 
    s=s@entry=0x7fffffffd850) at upb.c:3497

(etc)

I'll work on a fix.

[haberman]

Fixed in #530