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

Calling inspect on a proto instance causes a NullPointerException on subsequent to_proto #9507

Closed
NC-piercej opened this issue Feb 15, 2022 · 6 comments · Fixed by #9637
Closed
Assignees
Labels
bug jruby Issues unique to the JRuby interpreter ruby

Comments

@NC-piercej
Copy link

NC-piercej commented Feb 15, 2022

What version of protobuf and what language are you using?
Version: v3.19.4
Language: JRuby

What operating system (Linux, Windows, ...) and version?

Linux, official JRuby 9.3.3.0 docker image (jruby:9.3.3.0-jre11)

What runtime / compiler are you using (e.g., python version or gcc version)

JRuby 9.3.3.0

What did you do?

This will work:

data = File.read("path/to/proto.proto", mode: "rb")
proto = SomeProtoClass.decode(data)
proto.to_proto

This will explode:

data = File.read("path/to/proto.proto", mode: "rb")
proto = SomeProtoClass.decode(data)
proto.inspect
proto.to_proto

What did you expect to see

Instance converted to proto binary without an error.

What did you see instead?

Java::JavaLang::NullPointerException: from
com.google.protobuf.jruby.RubyMessage.build(RubyMessage.java:680)","com.google.protobuf.jruby.RubyMessage.convert(RubyMessage.java:858)","com.google.protobuf.jruby.RubyMessage.build(RubyMessage.java:663)","com.google.protobuf.jruby.RubyMessage.build(RubyMessage.java:639)","com.google.protobuf.jruby.RubyMessage.encode(RubyMessage.java:475)","com.google.protobuf.jruby.RubyMessage$INVOKER$s$1$0$encode.call(RubyMessage$INVOKER$s$1$0$encode.gen)","org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:372)","org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:175)","org.jruby.dist/org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:316)","org.jruby.dist/org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)","org.jruby.dist/org.jruby.ir.interpreter.InterpreterEngine.interpret(InterpreterEngine.java:80)","org.jruby.dist/org.jruby.internal.runtime.methods.MixedModeIRMethod.INTERPRET_METHOD(MixedModeIRMethod.java:164)","org.jruby.dist/org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:151)","org.jruby.dist/org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:198)","org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:351)","org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:144)","org.jruby.dist/org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:345)","org.jruby.dist/org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:72)","org.jruby.dist/org.jruby.ir.interpreter.Interpreter.INTERPRET_BLOCK(Interpreter.java:116)","org.jruby.dist/org.jruby.runtime.MixedModeIRBlockBody.commonYieldPath(MixedModeIRBlockBody.java:136)","org.jruby.dist/org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:66)","org.jruby.dist/org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:58)","org.jruby.dist/org.jruby.runtime.Block.call(Block.java:143)","org.jruby.dist/org.jruby.RubyProc.call(RubyProc.java:309)","org.jruby.dist/org.jruby.internal.runtime.RubyRunnable.run(RubyRunnable.java:105)","java.base/java.lang.Thread.run(Unknown Source)"

Anything else we should know about your project / environment

Only some messages seem to have this issue. We also suspect that methods other than inspect may be able to trigger the issue.

This seems to only occur when decode is used to build the message from a proto binary string. When the proto is built using the constructor from a hash, everything works fine.

In general, the JRuby version has a ton of bugs relative to the CRuby version. Methods like dup plain do not work on the JRuby version (I may create a separate issue about this).

@JasonLunn
Copy link
Contributor

@NC-piercej - Only some messages seem to have this issue - can you attach an example proto that reproduces the issue?

We use .dup in our tests - please file a separate issue detailing how to reproduce a failure.

@JasonLunn JasonLunn self-assigned this Feb 25, 2022
@JasonLunn JasonLunn added the jruby Issues unique to the JRuby interpreter label Feb 25, 2022
@NC-piercej
Copy link
Author

NC-piercej commented Feb 25, 2022

@JasonLunn Here is an example:

#!/usr/bin/ruby

require 'google/protobuf'
require 'test/unit'

class InspectTest < Test::Unit::TestCase
  def test_type_enum_not_set_with_other_field_set_breaks_during_encode
    m = NpeMessage.new(
    # type: :Something, # doesn't have to be set
      other: "foo"      # must be set, but can be blank
    )
    
    begin
      encoded = NpeMessage.encode(m)
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end

    decoded = NpeMessage.decode(encoded)
    decoded.inspect
    decoded.to_proto
  end

  def test_type_enum_with_other_field_set_breaks_after_inspect
    m = NpeMessage.new(
      type: :Something,     # can be set
      other: "foo"          # must be set, and cannot be blank
    )
    
    encoded = NpeMessage.encode(m)
    decoded = NpeMessage.decode(encoded)
    decoded.inspect
    begin
      decoded.to_proto
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end
  end

  def test_type_enum_with_other_field_not_set_works
    m = NpeMessage.new(
      type: :Something,     # can be set
    # other: ""             # shouldn’t be set to demonstrate error
    )
    
    begin
      encoded = NpeMessage.encode(m)
      decoded = NpeMessage.decode(encoded)
      decoded.inspect
      decoded.to_proto
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end
  end

  def test_nontype_enum_with_other_field_works
    m = WorkingNonTypeEnumMessage.new(
      nontype: :Something,
      other: "something else"
    )
    
    begin
      encoded = WorkingNonTypeEnumMessage.encode(m)
      decoded = WorkingNonTypeEnumMessage.decode(encoded)
      decoded.inspect
      decoded.to_proto
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end
  end

  def test_type_enum_without_other_field_works
    m = WorkingTypeEnumMessage.new(
      type: :Something
    )
    
    begin
      encoded = WorkingTypeEnumMessage.encode(m)
      decoded = WorkingTypeEnumMessage.decode(encoded)
      decoded.inspect
      decoded.to_proto
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end
  end

  def test_type_non_enum_with_other_field_works
    m = WorkingTypeNonEnumMessage.new(
      type: "something",
      other: "something else"
    )
    
    begin
      encoded = WorkingTypeNonEnumMessage.encode(m)
      decoded = WorkingTypeNonEnumMessage.decode(encoded)
      decoded.inspect
      decoded.to_proto
    rescue java.lang.NullPointerException
      flunk "NPE rescued"
    end
  end

  pool = Google::Protobuf::DescriptorPool.new
  pool.build do
    add_message "NpeMessage" do
      optional :type, :enum, 1, "TestEnum" 
      optional :other, :string, 2
    end

    add_message "WorkingNonTypeEnumMessage" do
      optional :nontype, :enum, 1, "TestEnum" 
      optional :other, :string, 2
    end

    add_message "WorkingTypeEnumMessage" do
      optional :type, :enum, 1, "TestEnum" 
    end

    add_message "WorkingTypeNonEnumMessage" do
      optional :type, :string, 1
      optional :other, :string, 2
    end

    add_enum "TestEnum" do
      value :Something, 0
    end
  end

  NpeMessage = pool.lookup("NpeMessage").msgclass
  WorkingNonTypeEnumMessage = pool.lookup("WorkingNonTypeEnumMessage").msgclass
  WorkingTypeEnumMessage = pool.lookup("WorkingTypeEnumMessage").msgclass
  WorkingTypeNonEnumMessage = pool.lookup("WorkingTypeNonEnumMessage").msgclass
  TestEnum = pool.lookup("TestEnum").enummodule
end

We have narrowed the issue down to an enum named type being present in the message. We're continuing to investigate and may put up a PR in the near future.

cc @ericksonb

@NC-piercej
Copy link
Author

We've further confirmed that the issue is related to an enum named type and that this issue is isolated to the JRuby version of the gem: the CRuby version is not affected.

@JasonLunn
Copy link
Contributor

@NC-piercej - can you confirm that you're experiencing this issue under JRuby 9.3.3.0 but not under JRuby 9.2.x?

@NC-piercej
Copy link
Author

@JasonLunn The issue seems to affect 9.2.x as well:

Screen Shot 2022-03-14 at 9 01 58 AM

@NC-piercej
Copy link
Author

Thanks so much for the fix, @JasonLunn! 🎉 😄

jorgbrown added a commit that referenced this issue Apr 2, 2022
Cherry pick JRuby changes that fix NPE during encoding (#9507) and implements respond_to? (#9202) into 3.20.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug jruby Issues unique to the JRuby interpreter ruby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants