Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Taking ownership of value captured by a lambda env is unsafe #1040

Open
TimDeve opened this issue Dec 1, 2020 · 5 comments · May be fixed by #1440
Open

Taking ownership of value captured by a lambda env is unsafe #1040

TimDeve opened this issue Dec 1, 2020 · 5 comments · May be fixed by #1440
Labels
bug haskell memory Borrow checker and lifetimes

Comments

@TimDeve
Copy link
Contributor

TimDeve commented Dec 1, 2020

The following example will result in a double free:

(defn main []
  (let [s @"String"
        f (fn [] s)]
    (ignore (f))))

The reason is that returning the value s means the consumer of the lambda will take ownership of it, so when it is done with the value it will call delete on it, but the value is still captured in the env of the lambda so when the env gets deleted it tries to free something already free-ed.

For the same reason calling a function that take ownership in the lambda will result in the same problem:

(defn main []
  (let [s @"String"
        f (fn [] (ignore s))]
    (f)))
@TimDeve
Copy link
Contributor Author

TimDeve commented Dec 1, 2020

Two solutions come to mind: the simplest is to disallow taking ownership of env values in lambdas. Alternatively we could remove the value from the env when it's consumed and only allow to call these lambda once, but it seems that would be quite complex to implement.

@scolsen
Copy link
Contributor

scolsen commented Oct 18, 2022

Two solutions come to mind: the simplest is to disallow taking ownership of env values in lambdas. Alternatively we could remove the value from the env when it's consumed and only allow to call these lambda once, but it seems that would be quite complex to implement.

The second solution would be "linear (or affine if we permit 0 usages) lambdas" and would probably be more robust. As mentioned in some other issues, I think this would mostly entail tracking linearity dependencies across values lexically referenced in multiple scopes. You could conceive as linearity/ownership as a transitive property, for things that take ownership of a reference via closure e.g. lambdas that capture some linear value can only be used once, structs that capture some linear value can only have that field referenced once, etc. and the lifetime of the captured thing should be incremented

another way to talk about this is that the lambda in this case "leaks" s by providing a means of accessing it beyond its assigned scope/lifetime. The most intuitive behavior would be for the linearity to be "transferred" to the lambda in the first case, (you can only call it once).

as a rough sketch, we could analyze this as:

[s @"String"<lt=1>
  f (fn [] s <lt=2>) <lt=1>

where a reference capture occurs iff the same value e.g. s is assigned a higher lifetime in a subscope, capture wouldn't occur, for example when:

[s @"String"<lt=1>
  t s <lt=1> <lt=1>

since the lifetime assignments for all instances of s are equal.

In the case of a capture, where some lifetime assignment greater than another lifetime assignment exists, we do not perform a deletion, but instead only add the dependency; e.g.

START_SCOPE_1
s @"String" ;; assign s.lifetime = 1
f (fn [] s <lt=2>) ;; assign f.lifetime = 1; s.lifetime++; f.dependent = s
END_SCOPE_1
;; check lifetimes:
;;     s.lifetime--; s.lifetime = 1; do nothing.
;;     f.lifetime--; -- delete f; s.lifetime--; delete s.

@TimDeve
Copy link
Contributor Author

TimDeve commented Oct 19, 2022

Yes, affine FnOnce-like lambdas is what I had in mind.

@scolsen
Copy link
Contributor

scolsen commented Oct 29, 2022

Been thinking about this a bit more.

In the current implementation we store the env as a struct ty, so its deleter is the same as that of a struct (it'll just call delete on all members unless they are not isManaged). The issue here is: we can't just remove the capture from the env, because then the lambda doesn't have access to it. Unfortunately, since we only determine deletions by types, we'll always wind up calling a deletion on the capture when we delete the lambda's env (also necessary).

The most robust solution would probably be to introduce a proper linear type that, similar to refs, includes a lifetime. A more adhoc solution is to just tag the captured variables with an additional flag that indicates whether or not they should be deleted, which effectively "overrides" the default behavior we currently use for finding deleters for types.

We could also introduce a separate lambda type, like FnOnce that behaves a bit differently.

In terms of the current implementation, simply disallowing returning captured memory would have the least impact. We could restrict lambda captures to references only.

@scolsen
Copy link
Contributor

scolsen commented Oct 29, 2022

I think only allowing reference captures might be the best approach w.r.t the current implementation. We already correctly track reference lifetimes:

(defn foo [] 
  (let [s @"foo"
          f (fn [] &s)])) ; error
(defn foo [] 
  (let [s @"foo"
          f (fn [] @&s)])) ; OK

Is there any use case that we'd have to permit ownership taking captures for?

EDIT: We also correctly catch the case:

(defn foo [] (let [s @"foo" f (fn [] &s)] f))
(defn bar [] ((foo)))
The reference '(defn bar [] ((foo)))' (depending on the variable '_5') isn't alive at line 6, column 2 in 'REPL'. at REPL:6:2.

Allowing for a "move and capture" in a robust way would require introducing a proper Linear Type kind that includes lifetime analysis for the linear values as we do with Refs currently, so it'd take some work. The current system is also simpler. I guess adding this restriction means you can't write a lambda that has a an allocation-free non-ref capture, but it's not impossible. You'd only have to convert a value to a pointer and then manually manage the memory. Since this is maybe an advanced use case anyway, perhaps that's OK?

scolsen added a commit to scolsen/Carp that referenced this issue Oct 30, 2022
When lambdas close over some external environment variable, if that
variable is a linear value, their environment becomes the owner of the
captured value and the value will be freed with the environment. If the
lambda moves this variable out of its own scope, however, e.g. by
returning it in its body, both the caller and the lambda environment
will wind up owning the value, resulting in double frees.

For now, we prevent this scenario by simply disallowing functions to
leak variables captured from another scope. Doing so is now an error. Of
course, one can still copy these values without issue.

We achieve this through set operations. If the memory state loses the
fake deleters for the set of a lambdas captured bindings after its body
is evaluated, we've encountered an ownership leak, and report an error.

```clojure
(defn example []
  (let [capture @""
        lambda (fn [] capture)])) ;; this is now an error

(defn example []
  (let [capture @""
        lambda (fn [] @&capture)])) ;; this is still OK
```

fixes carp-lang#1040
@scolsen scolsen linked a pull request Oct 30, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug haskell memory Borrow checker and lifetimes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants