Use SetFinalizer to avoid leaking Snapshots.

Use SetFinalizer to avoid leaking Snapshots.

R=dnj@chromium.org, martiniss@chromium.org, vadimsh@chromium.org
BUG=

Committed: https://chromium.googlesource.com/infra/infra/+/efa0090f0d734750af5ebd4822e2585362b6fb4c

[Vadim Sh., 2015-07-13 18:36:47]

https://codereview.chromium.org/1223263006/diff/1/go/src/infra/gae/libs/gae/m...
File go/src/infra/gae/libs/gae/memory/gkvlite_utils.go (right):

https://codereview.chromium.org/1223263006/diff/1/go/src/infra/gae/libs/gae/m...
go/src/infra/gae/libs/gae/memory/gkvlite_utils.go:81: go s.Close()
why not make Close a part of public interface of Store and require callers to
Close it (as is usually done in Go to control object life time)?

[iannucci, 2015-07-13 18:41:13]

Because that would require me to make Close part of the DSIterator
interface, which it's not in the SDK. then id have to provide leak
detection in the tests, etc...

On Mon, Jul 13, 2015, 11:36  <vadimsh@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1223263006/diff/1/go/src/infra/gae/libs/gae/m...
> File go/src/infra/gae/libs/gae/memory/gkvlite_utils.go (right):
>
>
>
https://codereview.chromium.org/1223263006/diff/1/go/src/infra/gae/libs/gae/m...
> go/src/infra/gae/libs/gae/memory/gkvlite_utils.go:81
>
<https://codereview.chromium.org/1223263006/diff/1/go/src/infra/gae/libs/gae/m...>:
> go s.Close()
> why not make Close a part of public interface of Store and require
> callers to Close it (as is usually done in Go to control object life
> time)?
>
> https://codereview.chromium.org/1223263006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Vadim Sh., 2015-07-13 18:44:20]

lgtm

I just try to understand why this is unavoidable. How does "real" (non-mock)
implementation deal with this problem? Is it using finalizer internally?

[iannucci, 2015-07-13 18:46:19]

It does finalization in the servers when the rpc socket closes

On Mon, Jul 13, 2015, 11:44  <vadimsh@chromium.org> wrote:

> lgtm
>
> I just try to understand why this is unavoidable. How does "real"
> (non-mock)
> implementation deal with this problem? Is it using finalizer internally?
>
> https://codereview.chromium.org/1223263006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[iannucci, 2015-07-13 18:50:28]

(It's also eventually consistent, I think, whereas this is a bit more
precise because it's not distributed)

On Mon, Jul 13, 2015, 11:46 Robert Iannucci <iannucci@chromium.org> wrote:

> It does finalization in the servers when the rpc socket closes
>
> On Mon, Jul 13, 2015, 11:44  <vadimsh@chromium.org> wrote:
>
>> lgtm
>>
>> I just try to understand why this is unavoidable. How does "real"
>> (non-mock)
>> implementation deal with this problem? Is it using finalizer internally?
>>
>> https://codereview.chromium.org/1223263006/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[iannucci, 2015-07-14 00:00:07]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2015-07-14 00:00:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1223263006/1

[commit-bot: I haz the power, 2015-07-14 00:02:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-14 00:02:30]

Try jobs failed on following builders:
  infra_tester on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/infra_tester/bu...)

[iannucci, 2015-07-14 00:03:26]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2015-07-14 00:03:27]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://codereview.chromium.org/1223263006/#ps20001
(title: "rebase on origin/master")

[commit-bot: I haz the power, 2015-07-14 00:03:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1223263006/20001

[commit-bot: I haz the power, 2015-07-14 00:06:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/infra/infra/+/efa0090f0d734750af5ebd4822e25...