[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427583#comment-15427583 ] ASF GitHub Bot commented on MESOS-6038: --- Github user aaronjwood closed the pull request at: https://github.com/apache/mesos/pull/157 > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15427582#comment-15427582 ] ASF GitHub Bot commented on MESOS-6038: --- Github user aaronjwood commented on the issue: https://github.com/apache/mesos/pull/157 It looks like I was wrong about this, I had thought the copy constructor of tuple was getting called when it wasn't the copy constructor at all. I did some more testing with this and found that the original version behaves properly. > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15425075#comment-15425075 ] ASF GitHub Bot commented on MESOS-6038: --- Github user haosdent commented on the issue: https://github.com/apache/mesos/pull/157 You could join via https://mesos-slackin.herokuapp.com/ > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15425055#comment-15425055 ] ASF GitHub Bot commented on MESOS-6038: --- Github user aaronjwood commented on the issue: https://github.com/apache/mesos/pull/157 I don't have access to their Slack but I agree. > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15425019#comment-15425019 ] ASF GitHub Bot commented on MESOS-6038: --- Github user haosdent commented on the issue: https://github.com/apache/mesos/pull/157 By the way, copy a comment from @jpeach in https://mesos.slack.com/messages/cxx/ ``` Promise promise = Promise(); why not just ```Promise promise;``` `` > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15425009#comment-15425009 ] ASF GitHub Bot commented on MESOS-6038: --- Github user aaronjwood commented on the issue: https://github.com/apache/mesos/pull/157 No no, that's okay. I appreciate everyone helping me validate these changes. I'm trying to be very careful with this :) > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15424995#comment-15424995 ] ASF GitHub Bot commented on MESOS-6038: --- Github user haosdent commented on the issue: https://github.com/apache/mesos/pull/157 >Even though the Promise is local to the stack, when we return Future shouldn't it be a copy of what's needed underneath? Sorry, I misunderstanding something before. Future store its data in `std::shared_ptr data` and your function return a copy instead of const reference, so it should work. > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15424946#comment-15424946 ] ASF GitHub Bot commented on MESOS-6038: --- Github user aaronjwood commented on the issue: https://github.com/apache/mesos/pull/157 I might be misunderstanding things but why can we not do this? I see it being done here https://github.com/apache/mesos/blob/master/3rdparty/libprocess/src/http.cpp#L1001 If we make the `Promise` local like it is in this patch, pass its address into the tuple via the copy constructor (that should be the copy constructor getting called, right?) so that it takes a copy and doesn't rely on an actual address from the stack, and return the future in the same way as the example above, would that not work? Even though the `Promise` is local to the stack, when we return `Future` shouldn't it be a copy of what's needed underneath? > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15424836#comment-15424836 ] ASF GitHub Bot commented on MESOS-6038: --- Github user haosdent commented on the issue: https://github.com/apache/mesos/pull/157 +1 For what @Vish0007 said. If promise became a locale scope object here, return its future seems incorrect. I search `return \S+\.future\(\);` in all exists code, all `Promise` are member objects or heap objects. > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15424716#comment-15424716 ] ASF GitHub Bot commented on MESOS-6038: --- Github user Vish0007 commented on the issue: https://github.com/apache/mesos/pull/157 @aaronjwood the "promise" object you have has local scope (and lifetime) and its not valid outside the method; I am sure valgrind wold report this as error; > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MESOS-6038) Prevent leaking allocated memory for Promises
[ https://issues.apache.org/jira/browse/MESOS-6038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15419975#comment-15419975 ] Aaron Wood commented on MESOS-6038: --- Looking for a shepherd to help review this :) > Prevent leaking allocated memory for Promises > - > > Key: MESOS-6038 > URL: https://issues.apache.org/jira/browse/MESOS-6038 > Project: Mesos > Issue Type: Bug > Components: c++ api, libprocess >Reporter: Aaron Wood >Priority: Minor > Labels: github-import, patch > > I had found a few places where these Promise objects were leaking. Once the > Future was returned there was no way to free the memory for the Promise > object that had been allocated thus causing the leak. > https://reviews.apache.org/r/51068/ > https://github.com/apache/mesos/pull/157 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
