>From mesos.pb.h/mesos.pb.cc

```
  inline ContainerID& operator=(const ContainerID& from) {
    CopyFrom(from);
    return *this;
  }

void ContainerID::CopyFrom(const ::google::protobuf::Message& from) {
  if (&from == this) return;
  Clear();
  MergeFrom(from);
}
```

On Mon, Sep 19, 2016 at 4:32 PM, haosdent <haosd...@gmail.com> wrote:

> @Neil, when
>
> ```
> rootContainerId = rootContainerId.parent();
> ```
>
> protobuf would try to call `ContaienrID::Clear()` first and then perform
> `ContainerID::CopyFrom`. Because the parent has been broken after
> `ContainerID::Clear()`, so the `ContainerID::CopyFrom` would get an empty
> value. I think this is not a bug of protobuf and we should avoid using
>
> ```
> Message = Message.xx();
> ```
>
> On Mon, Sep 19, 2016 at 3:42 PM, Neil Conway <neil.con...@gmail.com>
> wrote:
>
>> Hi Jie,
>>
>> Do you have more details on what exactly the problem is here? If
>> protobuf is unable to copy/merge nested messages in general, that
>> seems like something that might crop up elsewhere.
>>
>> Perhaps we can (a) file a JIRA (ideally with a self-contained
>> test-case), and/or (c) report the problem to upstream?
>>
>> Neil
>>
>> ---------- Forwarded message ----------
>> From:  <ji...@apache.org>
>> Date: Sat, Sep 17, 2016 at 11:27 PM
>> Subject: mesos git commit: Fixed a bug in getRootContainerId due to
>> protobuf copying issue.
>> To: comm...@mesos.apache.org
>>
>>
>> Repository: mesos
>> Updated Branches:
>>   refs/heads/master a4fd86bce -> be81a924a
>>
>>
>> Fixed a bug in getRootContainerId due to protobuf copying issue.
>>
>> It looks like protobuf is not so great dealing with nesting messages
>> when doing merge or copy. This patch uses an extra copy to bypass that
>> issue in the protobuf.
>>
>> Review: https://reviews.apache.org/r/51992
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/be81a924
>> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/be81a924
>> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/be81a924
>>
>> Branch: refs/heads/master
>> Commit: be81a924a9e9414ec98f8c9a87a5391dad865146
>> Parents: a4fd86b
>> Author: Jie Yu <yujie....@gmail.com>
>> Authored: Sat Sep 17 14:22:31 2016 -0700
>> Committer: Jie Yu <yujie....@gmail.com>
>> Committed: Sat Sep 17 14:25:33 2016 -0700
>>
>> ----------------------------------------------------------------------
>>  src/slave/containerizer/mesos/utils.hpp | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/mesos/blob/be81a924/s
>> rc/slave/containerizer/mesos/utils.hpp
>> ----------------------------------------------------------------------
>> diff --git a/src/slave/containerizer/mesos/utils.hpp
>> b/src/slave/containerizer/mesos/utils.hpp
>> index 2bb55c1..178ebf3 100644
>> --- a/src/slave/containerizer/mesos/utils.hpp
>> +++ b/src/slave/containerizer/mesos/utils.hpp
>> @@ -27,7 +27,12 @@ static ContainerID getRootContainerId(const
>> ContainerID& containerId)
>>  {
>>    ContainerID rootContainerId = containerId;
>>    while (rootContainerId.has_parent()) {
>> -    rootContainerId = rootContainerId.parent();
>> +    // NOTE: Looks like protobuf does not handle copying well when
>> +    // nesting message is involved, because the source and the target
>> +    // point to the same object. Therefore, we create a temporary
>> +    // variable and use an extra copy here.
>> +    ContainerID id = rootContainerId.parent();
>> +    rootContainerId = id;
>>    }
>>
>>    return rootContainerId;
>>
>
>
>
> --
> Best Regards,
> Haosdent Huang
>



-- 
Best Regards,
Haosdent Huang

Reply via email to