starwing / lua-protobuf

A Lua module to work with Google protobuf
MIT License
1.75k stars 387 forks source link

Fix path processing #190

Open fmaksim74 opened 2 years ago

fmaksim74 commented 2 years ago

Redundant check of "self" variable breaks search path processing. It create new instance of Parser and discards all paths added with "addpath" method.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.001%) to 99.652% when pulling f43a275cdb8f20101d4918ef1f7292da2676aa6c on fmaksim74:fix-path-processing into 6ffd7a255b024d1367574e33da3ad0eb6ca8d1fc on starwing:master.

starwing commented 2 years ago

this behavior is on purpose.

Someone may use require "protoc":load [[...]] to load a protobuf schema into pb module. But that is only permitted in demo usage. Because in this case the table version of schema may get stuck in memory forever. So a commit added at https://github.com/starwing/lua-protobuf/commit/cb977e421e1e70a61a278e9f93e00e2e2b940f94 to force use proto.new() if anyone wants load schema multiple times. The parser created by protoc:load() usage will drop immediately.

fmaksim74 commented 2 years ago

I checked this case:

print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))

it works fine. require "protoc" returns valid pointer to Parser.

starwing commented 2 years ago

Yes, but this example means the Phone/Person tables are anchored to Lua memory and can not collect i.e. the memory leak. and it prevent you hotfix the message (you can not load the same name of message again):

print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))
print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))
true    145
lua: .\protoc.lua:82: <input>:1:18: type .Phone already defined
stack traceback:
        [C]: in function 'error'
        .\protoc.lua:82: in function <.\protoc.lua:80>
        (...tail calls...)
        .\protoc.lua:630: in local 'top_parser'
        .\protoc.lua:851: in function 'protoc.parse'
        .\protoc.lua:1162: in upvalue 'do_compile'
        .\protoc.lua:1166: in function 'protoc.compile'
        .\protoc.lua:1176: in function 'protoc.load'
        tt.lua:15: in main chunk
        [C]: in ?
sonarcloud[bot] commented 11 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information