Re: [racket-dev] [plt] Push #27825: master branch updated
+1 for Robby. On Nov 26, 2013, at 12:05 PM, Robby Findler wrote: > This isn't the semantics of the limits. It is about the way that calling the > evaluator interacts with threads. Changing this program (from Eli's email) > > -> ,r racket/sandbox > -> (define e (make-evaluator 'racket)) > -> (e '(define foo 1)) > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > > so that that final call doesn't return until the thread terminates is a huge > change of the semantics and not at all what a reasonable reading of the docs > would suggest. The docs for make-evaluator don't specifically mention > 'thread' but it seems pretty clear to me that when you pass a program that > creates a thread to an evaluator, the call returns without waiting for the > thread to terminate. > > I also agree with Eli that this would break essentially all uses of threads > in sandboxes. > > Robby > > > > > On Tue, Nov 26, 2013 at 10:57 AM, Jay McCarthy wrote: > How can you change the documentation to say anything other than > "Wherever you see an thunk having a time restriction, don't think that > that means the thunk is restricted to using only a certain amount of > time"? We'd have to say something like "Time in this content is a > -first-order- notion of time, despite being in Racket where we > generally try to go out of our way to make sure everything works in > every higher order context." My position is that racket/sandbox has > always been broken and that any reasonable reader of the documentation > would have expected this behavior all along. (In fact, both Matthew > and I did think it did this and were surprised that it didn't.) > > On Tue, Nov 26, 2013 at 9:54 AM, Robby Findler > wrote: > > I think that, in this case, changing the documentation and adding the > > functionality with a different name is the way to go. > > > > Robby > > > > > > > > On Tue, Nov 26, 2013 at 10:49 AM, Jay McCarthy wrote: > >> > >> I agree that it is different. > >> > >> I disagree that this is a problem. > >> > >> The documentation says that is executes the code with a time > >> restriction. This implies to me that (call-with-limits X #f t) should > >> not use more than X secs of resources, but it is trivial to produce > >> counter-examples. Without this change, call-with-limits is totally > >> useless for limiting the time taken by untrustworthy code. > >> > >> The fact that there was no test case that failed with my change tells > >> me that the code was not written to make one decision or another. I > >> charitably assume that this was because the good (current) behavior is > >> what was wanted but the variety of attacks on it were not thought of. > >> > >> Nevertheless, if you and Matthew think this is a bad change, we should > >> change everywhere in racket/sandbox that mentions time restrictions to > >> clarify that they don't actually restrict time of the code. > >> > >> Jay > >> > >> > >> > >> On Mon, Nov 25, 2013 at 3:02 AM, Eli Barzilay wrote: > >> > IIUC, this makes the limit thing -- and therefore sandboxes -- behave > >> > *very* differently. The original intention was that the time limit is > >> > on something similar to what you get with `time'. > >> > > >> > A very visible way to see the effect of this change: > >> > > >> > -> ,r racket/sandbox > >> > -> (define e (make-evaluator 'racket)) > >> > -> (e '(define foo 1)) > >> > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > >> > # > >> > > >> > This used to happen immediately, with the thread continuing to run > >> > inside the sandbox. After your change, the last line hangs until the > >> > thread is done. Using a bigger sleeping time will make it throw an > >> > error when it previously didn't. Similarly, > >> > > >> > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") > >> > > >> > used to work and will throw an error now, and of course, any code that > >> > runs some kind of sandboxed server will probably break now. > >> > > >> > I think that instead of this, it'd be better to write a helper that > >> > runs a thunk and waits for it and for any generated threads to end, > >> > and suggest using this helper when you want to wait for all threads in > >> > a `with-limits'. (It might also be useful in the profiler, where a > >> > similar kind of wait-for-all is done.) > >> > > >> > > >> > On Friday, j...@racket-lang.org wrote: > >> >> jay has updated `master' from e0026f5de4 to 79f8636e1e. > >> >> http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e > >> >> > >> >> =[ One Commit > >> >> ]= > >> >> Directory summary: > >> >> 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ > >> >> 45.6% pkgs/sandbox-lib/racket/ > >> >> > >> >> ~~ > >> >> > >> >> 79f8636 Jay McCarthy 2013-11-22 14:25 > >> >> : > >> >> | Ensure that threads created within call-with-limits are accounted > >> >> | during the time/space limits > >> >> : > >> >>
Re: [racket-dev] [plt] Push #27825: master branch updated
This isn't the semantics of the limits. It is about the way that calling the evaluator interacts with threads. Changing this program (from Eli's email) -> ,r racket/sandbox -> (define e (make-evaluator 'racket)) -> (e '(define foo 1)) -> (e '(thread (lambda () (sleep 5) (set! foo 2 so that that final call doesn't return until the thread terminates is a huge change of the semantics and not at all what a reasonable reading of the docs would suggest. The docs for make-evaluator don't specifically mention 'thread' but it seems pretty clear to me that when you pass a program that creates a thread to an evaluator, the call returns without waiting for the thread to terminate. I also agree with Eli that this would break essentially all uses of threads in sandboxes. Robby On Tue, Nov 26, 2013 at 10:57 AM, Jay McCarthy wrote: > How can you change the documentation to say anything other than > "Wherever you see an thunk having a time restriction, don't think that > that means the thunk is restricted to using only a certain amount of > time"? We'd have to say something like "Time in this content is a > -first-order- notion of time, despite being in Racket where we > generally try to go out of our way to make sure everything works in > every higher order context." My position is that racket/sandbox has > always been broken and that any reasonable reader of the documentation > would have expected this behavior all along. (In fact, both Matthew > and I did think it did this and were surprised that it didn't.) > > On Tue, Nov 26, 2013 at 9:54 AM, Robby Findler > wrote: > > I think that, in this case, changing the documentation and adding the > > functionality with a different name is the way to go. > > > > Robby > > > > > > > > On Tue, Nov 26, 2013 at 10:49 AM, Jay McCarthy > wrote: > >> > >> I agree that it is different. > >> > >> I disagree that this is a problem. > >> > >> The documentation says that is executes the code with a time > >> restriction. This implies to me that (call-with-limits X #f t) should > >> not use more than X secs of resources, but it is trivial to produce > >> counter-examples. Without this change, call-with-limits is totally > >> useless for limiting the time taken by untrustworthy code. > >> > >> The fact that there was no test case that failed with my change tells > >> me that the code was not written to make one decision or another. I > >> charitably assume that this was because the good (current) behavior is > >> what was wanted but the variety of attacks on it were not thought of. > >> > >> Nevertheless, if you and Matthew think this is a bad change, we should > >> change everywhere in racket/sandbox that mentions time restrictions to > >> clarify that they don't actually restrict time of the code. > >> > >> Jay > >> > >> > >> > >> On Mon, Nov 25, 2013 at 3:02 AM, Eli Barzilay wrote: > >> > IIUC, this makes the limit thing -- and therefore sandboxes -- behave > >> > *very* differently. The original intention was that the time limit is > >> > on something similar to what you get with `time'. > >> > > >> > A very visible way to see the effect of this change: > >> > > >> > -> ,r racket/sandbox > >> > -> (define e (make-evaluator 'racket)) > >> > -> (e '(define foo 1)) > >> > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > >> > # > >> > > >> > This used to happen immediately, with the thread continuing to run > >> > inside the sandbox. After your change, the last line hangs until the > >> > thread is done. Using a bigger sleeping time will make it throw an > >> > error when it previously didn't. Similarly, > >> > > >> > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") > >> > > >> > used to work and will throw an error now, and of course, any code that > >> > runs some kind of sandboxed server will probably break now. > >> > > >> > I think that instead of this, it'd be better to write a helper that > >> > runs a thunk and waits for it and for any generated threads to end, > >> > and suggest using this helper when you want to wait for all threads in > >> > a `with-limits'. (It might also be useful in the profiler, where a > >> > similar kind of wait-for-all is done.) > >> > > >> > > >> > On Friday, j...@racket-lang.org wrote: > >> >> jay has updated `master' from e0026f5de4 to 79f8636e1e. > >> >> http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e > >> >> > >> >> =[ One Commit > >> >> ]= > >> >> Directory summary: > >> >> 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ > >> >> 45.6% pkgs/sandbox-lib/racket/ > >> >> > >> >> ~~ > >> >> > >> >> 79f8636 Jay McCarthy 2013-11-22 14:25 > >> >> : > >> >> | Ensure that threads created within call-with-limits are accounted > >> >> | during the time/space limits > >> >> : > >> >> A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt > >> >> M pkgs/sandbox-lib/racket/sandbox.rkt | 81
Re: [racket-dev] [plt] Push #27825: master branch updated
How can you change the documentation to say anything other than "Wherever you see an thunk having a time restriction, don't think that that means the thunk is restricted to using only a certain amount of time"? We'd have to say something like "Time in this content is a -first-order- notion of time, despite being in Racket where we generally try to go out of our way to make sure everything works in every higher order context." My position is that racket/sandbox has always been broken and that any reasonable reader of the documentation would have expected this behavior all along. (In fact, both Matthew and I did think it did this and were surprised that it didn't.) On Tue, Nov 26, 2013 at 9:54 AM, Robby Findler wrote: > I think that, in this case, changing the documentation and adding the > functionality with a different name is the way to go. > > Robby > > > > On Tue, Nov 26, 2013 at 10:49 AM, Jay McCarthy wrote: >> >> I agree that it is different. >> >> I disagree that this is a problem. >> >> The documentation says that is executes the code with a time >> restriction. This implies to me that (call-with-limits X #f t) should >> not use more than X secs of resources, but it is trivial to produce >> counter-examples. Without this change, call-with-limits is totally >> useless for limiting the time taken by untrustworthy code. >> >> The fact that there was no test case that failed with my change tells >> me that the code was not written to make one decision or another. I >> charitably assume that this was because the good (current) behavior is >> what was wanted but the variety of attacks on it were not thought of. >> >> Nevertheless, if you and Matthew think this is a bad change, we should >> change everywhere in racket/sandbox that mentions time restrictions to >> clarify that they don't actually restrict time of the code. >> >> Jay >> >> >> >> On Mon, Nov 25, 2013 at 3:02 AM, Eli Barzilay wrote: >> > IIUC, this makes the limit thing -- and therefore sandboxes -- behave >> > *very* differently. The original intention was that the time limit is >> > on something similar to what you get with `time'. >> > >> > A very visible way to see the effect of this change: >> > >> > -> ,r racket/sandbox >> > -> (define e (make-evaluator 'racket)) >> > -> (e '(define foo 1)) >> > -> (e '(thread (lambda () (sleep 5) (set! foo 2 >> > # >> > >> > This used to happen immediately, with the thread continuing to run >> > inside the sandbox. After your change, the last line hangs until the >> > thread is done. Using a bigger sleeping time will make it throw an >> > error when it previously didn't. Similarly, >> > >> > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") >> > >> > used to work and will throw an error now, and of course, any code that >> > runs some kind of sandboxed server will probably break now. >> > >> > I think that instead of this, it'd be better to write a helper that >> > runs a thunk and waits for it and for any generated threads to end, >> > and suggest using this helper when you want to wait for all threads in >> > a `with-limits'. (It might also be useful in the profiler, where a >> > similar kind of wait-for-all is done.) >> > >> > >> > On Friday, j...@racket-lang.org wrote: >> >> jay has updated `master' from e0026f5de4 to 79f8636e1e. >> >> http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e >> >> >> >> =[ One Commit >> >> ]= >> >> Directory summary: >> >> 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ >> >> 45.6% pkgs/sandbox-lib/racket/ >> >> >> >> ~~ >> >> >> >> 79f8636 Jay McCarthy 2013-11-22 14:25 >> >> : >> >> | Ensure that threads created within call-with-limits are accounted >> >> | during the time/space limits >> >> : >> >> A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt >> >> M pkgs/sandbox-lib/racket/sandbox.rkt | 81 >> >> ++-- >> >> M .../racket-test/tests/racket/sandbox.rktl | 48 >> >> M .../scribblings/reference/sandbox.scrbl | 4 + >> > >> > -- >> > ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay: >> > http://barzilay.org/ Maze is Life! >> >> _ >> Racket Developers list: >> http://lists.racket-lang.org/dev > > _ Racket Developers list: http://lists.racket-lang.org/dev
Re: [racket-dev] [plt] Push #27825: master branch updated
I think that, in this case, changing the documentation and adding the functionality with a different name is the way to go. Robby On Tue, Nov 26, 2013 at 10:49 AM, Jay McCarthy wrote: > I agree that it is different. > > I disagree that this is a problem. > > The documentation says that is executes the code with a time > restriction. This implies to me that (call-with-limits X #f t) should > not use more than X secs of resources, but it is trivial to produce > counter-examples. Without this change, call-with-limits is totally > useless for limiting the time taken by untrustworthy code. > > The fact that there was no test case that failed with my change tells > me that the code was not written to make one decision or another. I > charitably assume that this was because the good (current) behavior is > what was wanted but the variety of attacks on it were not thought of. > > Nevertheless, if you and Matthew think this is a bad change, we should > change everywhere in racket/sandbox that mentions time restrictions to > clarify that they don't actually restrict time of the code. > > Jay > > > > On Mon, Nov 25, 2013 at 3:02 AM, Eli Barzilay wrote: > > IIUC, this makes the limit thing -- and therefore sandboxes -- behave > > *very* differently. The original intention was that the time limit is > > on something similar to what you get with `time'. > > > > A very visible way to see the effect of this change: > > > > -> ,r racket/sandbox > > -> (define e (make-evaluator 'racket)) > > -> (e '(define foo 1)) > > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > > # > > > > This used to happen immediately, with the thread continuing to run > > inside the sandbox. After your change, the last line hangs until the > > thread is done. Using a bigger sleeping time will make it throw an > > error when it previously didn't. Similarly, > > > > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") > > > > used to work and will throw an error now, and of course, any code that > > runs some kind of sandboxed server will probably break now. > > > > I think that instead of this, it'd be better to write a helper that > > runs a thunk and waits for it and for any generated threads to end, > > and suggest using this helper when you want to wait for all threads in > > a `with-limits'. (It might also be useful in the profiler, where a > > similar kind of wait-for-all is done.) > > > > > > On Friday, j...@racket-lang.org wrote: > >> jay has updated `master' from e0026f5de4 to 79f8636e1e. > >> http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e > >> > >> =[ One Commit ]= > >> Directory summary: > >> 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ > >> 45.6% pkgs/sandbox-lib/racket/ > >> > >> ~~ > >> > >> 79f8636 Jay McCarthy 2013-11-22 14:25 > >> : > >> | Ensure that threads created within call-with-limits are accounted > >> | during the time/space limits > >> : > >> A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt > >> M pkgs/sandbox-lib/racket/sandbox.rkt | 81 > ++-- > >> M .../racket-test/tests/racket/sandbox.rktl | 48 > >> M .../scribblings/reference/sandbox.scrbl | 4 + > > > > -- > > ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay: > > http://barzilay.org/ Maze is Life! > > _ > Racket Developers list: > http://lists.racket-lang.org/dev > _ Racket Developers list: http://lists.racket-lang.org/dev
Re: [racket-dev] [plt] Push #27825: master branch updated
I agree that it is different. I disagree that this is a problem. The documentation says that is executes the code with a time restriction. This implies to me that (call-with-limits X #f t) should not use more than X secs of resources, but it is trivial to produce counter-examples. Without this change, call-with-limits is totally useless for limiting the time taken by untrustworthy code. The fact that there was no test case that failed with my change tells me that the code was not written to make one decision or another. I charitably assume that this was because the good (current) behavior is what was wanted but the variety of attacks on it were not thought of. Nevertheless, if you and Matthew think this is a bad change, we should change everywhere in racket/sandbox that mentions time restrictions to clarify that they don't actually restrict time of the code. Jay On Mon, Nov 25, 2013 at 3:02 AM, Eli Barzilay wrote: > IIUC, this makes the limit thing -- and therefore sandboxes -- behave > *very* differently. The original intention was that the time limit is > on something similar to what you get with `time'. > > A very visible way to see the effect of this change: > > -> ,r racket/sandbox > -> (define e (make-evaluator 'racket)) > -> (e '(define foo 1)) > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > # > > This used to happen immediately, with the thread continuing to run > inside the sandbox. After your change, the last line hangs until the > thread is done. Using a bigger sleeping time will make it throw an > error when it previously didn't. Similarly, > > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") > > used to work and will throw an error now, and of course, any code that > runs some kind of sandboxed server will probably break now. > > I think that instead of this, it'd be better to write a helper that > runs a thunk and waits for it and for any generated threads to end, > and suggest using this helper when you want to wait for all threads in > a `with-limits'. (It might also be useful in the profiler, where a > similar kind of wait-for-all is done.) > > > On Friday, j...@racket-lang.org wrote: >> jay has updated `master' from e0026f5de4 to 79f8636e1e. >> http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e >> >> =[ One Commit ]= >> Directory summary: >> 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ >> 45.6% pkgs/sandbox-lib/racket/ >> >> ~~ >> >> 79f8636 Jay McCarthy 2013-11-22 14:25 >> : >> | Ensure that threads created within call-with-limits are accounted >> | during the time/space limits >> : >> A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt >> M pkgs/sandbox-lib/racket/sandbox.rkt | 81 >> ++-- >> M .../racket-test/tests/racket/sandbox.rktl | 48 >> M .../scribblings/reference/sandbox.scrbl | 4 + > > -- > ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay: > http://barzilay.org/ Maze is Life! _ Racket Developers list: http://lists.racket-lang.org/dev
Re: [racket-dev] [plt] Push #27825: master branch updated
Thanks! I think Jay and I became confused about the purpose of `call-with-limits` and thought it was supposed to constrain the time used by evaluation, no matter what it tries to do. We should revert the change, clarify the docs at `call-with-limits`, and maybe add something else to `racket/sandbox`. At Mon, 25 Nov 2013 05:02:07 -0500, Eli Barzilay wrote: > IIUC, this makes the limit thing -- and therefore sandboxes -- behave > *very* differently. The original intention was that the time limit is > on something similar to what you get with `time'. > > A very visible way to see the effect of this change: > > -> ,r racket/sandbox > -> (define e (make-evaluator 'racket)) > -> (e '(define foo 1)) > -> (e '(thread (lambda () (sleep 5) (set! foo 2 > # > > This used to happen immediately, with the thread continuing to run > inside the sandbox. After your change, the last line hangs until the > thread is done. Using a bigger sleeping time will make it throw an > error when it previously didn't. Similarly, > > (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") > > used to work and will throw an error now, and of course, any code that > runs some kind of sandboxed server will probably break now. > > I think that instead of this, it'd be better to write a helper that > runs a thunk and waits for it and for any generated threads to end, > and suggest using this helper when you want to wait for all threads in > a `with-limits'. (It might also be useful in the profiler, where a > similar kind of wait-for-all is done.) > > > On Friday, j...@racket-lang.org wrote: > > jay has updated `master' from e0026f5de4 to 79f8636e1e. > > http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e > > > > =[ One Commit ]= > > Directory summary: > > 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ > > 45.6% pkgs/sandbox-lib/racket/ > > > > ~~ > > > > 79f8636 Jay McCarthy 2013-11-22 14:25 > > : > > | Ensure that threads created within call-with-limits are accounted > > | during the time/space limits > > : > > A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt > > M pkgs/sandbox-lib/racket/sandbox.rkt | 81 > ++-- > > M .../racket-test/tests/racket/sandbox.rktl | 48 > > M .../scribblings/reference/sandbox.scrbl | 4 + > > -- > ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay: > http://barzilay.org/ Maze is Life! > > _ > Racket Developers list: > http://lists.racket-lang.org/dev _ Racket Developers list: http://lists.racket-lang.org/dev
Re: [racket-dev] [plt] Push #27825: master branch updated
IIUC, this makes the limit thing -- and therefore sandboxes -- behave *very* differently. The original intention was that the time limit is on something similar to what you get with `time'. A very visible way to see the effect of this change: -> ,r racket/sandbox -> (define e (make-evaluator 'racket)) -> (e '(define foo 1)) -> (e '(thread (lambda () (sleep 5) (set! foo 2 # This used to happen immediately, with the thread continuing to run inside the sandbox. After your change, the last line hangs until the thread is done. Using a bigger sleeping time will make it throw an error when it previously didn't. Similarly, (make-module-evaluator "#lang racket (thread (λ() (sleep 99)))") used to work and will throw an error now, and of course, any code that runs some kind of sandboxed server will probably break now. I think that instead of this, it'd be better to write a helper that runs a thunk and waits for it and for any generated threads to end, and suggest using this helper when you want to wait for all threads in a `with-limits'. (It might also be useful in the profiler, where a similar kind of wait-for-all is done.) On Friday, j...@racket-lang.org wrote: > jay has updated `master' from e0026f5de4 to 79f8636e1e. > http://git.racket-lang.org/plt/e0026f5de4..79f8636e1e > > =[ One Commit ]= > Directory summary: > 52.6% pkgs/racket-pkgs/racket-test/tests/racket/ > 45.6% pkgs/sandbox-lib/racket/ > > ~~ > > 79f8636 Jay McCarthy 2013-11-22 14:25 > : > | Ensure that threads created within call-with-limits are accounted > | during the time/space limits > : > A pkgs/racket-pkgs/racket-test/tests/racket/sandbox.rkt > M pkgs/sandbox-lib/racket/sandbox.rkt | 81 > ++-- > M .../racket-test/tests/racket/sandbox.rktl | 48 > M .../scribblings/reference/sandbox.scrbl | 4 + -- ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay: http://barzilay.org/ Maze is Life! _ Racket Developers list: http://lists.racket-lang.org/dev
