Failure to allocate memory when trying to change a message

[x-yuri]

What version of protobuf and what language are you using? Version: 3.22.3 Language: Ruby

What operating system (Linux, Windows, ...) and version? Arch Linux; Debian 10/11

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

What did you do?

a.sh:

v=ruby:2.7.8-slim-bullseye
v=ruby:3.0.0-slim-buster
v=ruby:3.2.2-slim-bullseye
docker run --rm -it "$v" sh -euc '
    mkdir app
    cd app
    cat > Gemfile <<EOF
        source "https://rubygems.org"
        gem "anycable", "1.3.0"
        gem "google-protobuf", "3.22.3"
EOF
    cat > a.rb <<EOF
        require "anycable/protos/rpc_pb"
        m = AnyCable::ConnectionRequest.new(env: {"headers" => {"A" => "a"}})
        encoded = AnyCable::ConnectionRequest.encode m
        p encoded
        decoded = AnyCable::ConnectionRequest.decode encoded
        p decoded
        p decoded.env.headers
        decoded.env.headers.delete("A")
        puts "done"
EOF
    bundle
    bundle exec ruby a.rb
'

$ sh -eu a.sh
...
"\x1A\b\x12\x06\n\x01A\x12\x01a"
<AnyCable::ConnectionRequest: env: <AnyCable::Env: url: "", headers: {"A"=>"a"}, cstate: {}, istate: {}>>
{"A"=>"a"}
a.rb: failed to allocate memory (NoMemoryError)

The issue manifests itself at least on Ruby 3.0.0..3.2.2. Without -it it somehow succeeds. I believe it can be reproduced not only in a docker container.

UPD Actually not only after decoding, this fails too:

m = AnyCable::ConnectionRequest.new(env: {"headers" => {"A" => "a"}})
m.env.headers.delete("A")

The protobuf files.

What did you expect to see No error and the program finishes correctly.

What did you see instead? The error and the program terminated.

Anything else we should know about your project / environment Affects google-protobuf-3.22.0..3.23.2 (but not under any conditions, i.e. under some conditions it might work).

[x-yuri]

I'm still investigating, and I have a more standalone case:

v=ruby:3.2.2-slim-bullseye
docker run --rm -it "$v" sh -euc '
    apt-get update
    apt-get install -y protobuf-compiler
    mkdir app
    cd app
    cat > Gemfile <<EOF
        source "https://rubygems.org"
        gem "google-protobuf", "3.22.3"
EOF
    cat > a.proto <<EOF
        syntax = "proto3";
        message MyMessage {
            map<string,string> my_map = 1;
        }
EOF
    mkdir out
    protoc a.proto --ruby_out out
    cat > a.rb <<EOF
        require "./out/a_pb"
        m = MyMessage.new(my_map: {"a" => "b"})
        m.my_map.delete("a")
        puts "done"
EOF
    bundle
    bundle exec ruby a.rb
'

Decided not to change what's already triaged.

With python expectedly it works:

v=python:3.11.3-slim-bullseye
docker run --rm -it "$v" sh -euc '
    apt-get update
    # apt-get install -y protobuf-compiler
    cat > a.proto <<EOF
        syntax = "proto3";
        message MyMessage {
            map<string,string> my_map = 1;
        }
EOF
    pip install protoc-exe
    protoc a.proto --python_out .
    pip install protobuf
    cat > a.py <<EOF
import a_pb2
m = a_pb2.MyMessage(my_map={"a": "b"})
del m.my_map["a"]
print("done")
EOF
    python a.py
'

[haberman]

Incredible repro, thanks for that.

I can confirm that the repro triggers NoMemoryError.

[x-yuri]

Incredible repro, thanks for that.

@haberman I just like filing bug reports :D

Long story short, you broke it in 3.22.0 when you were doing the 4-step change (adding the value parameter to upb_Map_Delete()).

https://github.com/protocolbuffers/upb/commit/3f173c4b810dd36c1ed24f4ffc82c1b2fb3eb60c \ https://github.com/protocolbuffers/protobuf/commit/b598b2dd1f00510dd7ebf0a075d55ef38c895172 \ https://github.com/protocolbuffers/upb/commit/0c6b72dbf891eafc91050ad60733ea3022fac2b3 \ https://github.com/protocolbuffers/protobuf/commit/b7d54ace5ea769a86da03cb65d3f97aa0580f71c

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/ruby-upb.c#L507-L512

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/map.c#L464-L473

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/ruby-upb.h#L994-L996

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/ruby-upb.h#L474-L486

In upb_Map_Delete() you change only the first 8 bytes of upb_MessageValue. Particularly, when self->value_type_info is kUpb_CType_String, upb_MessageValue contains upb_StringView:

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/ruby-upb.h#L422-L425

Which means size is left uninitialized (usually a very big number), and when it tries to convert it to a VALUE, it fails (I don't have so much RAM!!!:)):

https://github.com/protocolbuffers/protobuf/blob/482156c0f96dd670b9f1ba4f946e66edeb04535f/ruby/ext/google/protobuf_c/convert.c#L266-L270

Affects only ruby.

[haberman]

This was fixed in https://github.com/protocolbuffers/upb/pull/1341 and the fix will be released this week.