racket icon indicating copy to clipboard operation
racket copied to clipboard

fix sandbox GUI event handling

Open rmculpepper opened this issue 3 years ago • 5 comments

See https://stackoverflow.com/questions/72420079/why-is-my-sandboxed-racket-gui-app-not-responding-to-events/

Here is a simplified program that creates a sandbox and tries to create a GUI within the sandbox. On Racket 8.5, the frame appears frozen, because no GUI events are handled.

#lang racket
(require racket/sandbox
         racket/gui)
(define evaluator
  (call-with-trusted-sandbox-configuration
   (lambda () (make-evaluator 'racket/gui))))
(evaluator
 '(begin
    ;; (current-eventspace (make-eventspace)) ;; workaround
    (define frame (new frame% [label "Sandbox"]))
    (define msg (new message% [parent frame]
                     [label "No events so far..."]))
    (new button% [parent frame]
         [label "Click Me"]
         (callback (lambda (button event)
                     (send msg set-label "Button clicked"))))
    (send frame show #t)))
(sync never-evt)

The problem seems to be that in GUI mode, the sandbox creates an eventspace and runs the user evaluation loop in the eventspace's handler thread. But that loop blocks on a channel waiting for things to evaluate, which blocks the eventspace from handling any GUI events.

This PR changes the sandbox so that it runs the evaluation loop in an unrelated fresh thread. I'm not sure what the rationale for the original code was, so this might miss some property that it was aiming for.

rmculpepper avatar May 29 '22 14:05 rmculpepper

Maybe I'm missing something, but this doesn't sound right. Running GUI code in an unrelated thread means that you're manipulating GUI elements in a thread other than the handler thread, which is unlikely to be a good idea. The code in the Stack Overflow post would probably be ok running in the wrong thread, but it's easy to go wrong with a program that's only a little more complicated.

The advice I'd give to the OP is to explicitly yield. If there's no need to interactively evaluate after the program starts, then a (yield (current-eventspace)) at the end of the program or after starting it should do the right thing.

A possible improvement to the sandbox is to change (define expr (channel-get input-ch)) to (define expr (yield input-ch)) when running in a GUI namespace. That would let events get handled while waiting for an expression to evaluate. But that change might break existing uses that expect to be able to run a sequence of expressions before yeidling, so probably that improvement would have to be opt-in.

Meanwhile, it looks like the sandbox documentation doesn't explain that it uses a fresh eventspace's handler thread for evaluation, and that should be fixed.

mflatt avatar May 29 '22 14:05 mflatt

I don't understand that perspective. The GUI code in this example is the second example in the GUI manual, specifically in Creating Windows. If you run the code at the REPL, the GUI works fine. (I tested racket, racket -q, and racket -inq -l racket/base.) If you put the code in a module and run that, the GUI works fine. If you run the code in the DrRacket interactions window, the GUI works fine. But if you run it within a sandbox, the GUI is completely unresponsive.

rmculpepper avatar May 29 '22 16:05 rmculpepper

GUI programs work with a REPL because racket/gui sets current-get-interaction-input-port to do something analogous to (yield input-ch). DrRacket takes a slightly different approach of just using (yield) and new input calls queue-callback to trigger a read and evaluate, which is almost the same thing and at least the same in terms of concurrency. In both cases, REPL evaluation and GUI callbacks do not run in different threads, because that's such a bad idea.

If you take a Racket GUI program as a sequence of forms and feed them individually into a REPL, then a kind of atomicity across the sequence is lost, and things can go wrong compared to running the forms together as written. That's not exactly "the top level is hopeless", but it's a similar sort of compromise. Don't try to run Racket programs by piping them into a REPL's stdin. :)

The sandbox is REPL-like, and it could have been defined/implemented to behave even more like a REPL in this sense. Then, programs would have to used it under those constraints. The problem is that the sandbox hasn't been defined/implemented that way, and existing programs may well rely on the current behavior. The difference with (yield input-ch) would be subtle and not affect a lot of things, but introducing concurrency where it wasn't before is certainly asking for trouble. Being able to opt into more REPL-like behavior for a sandbox seems fine and useful.

mflatt avatar May 29 '22 16:05 mflatt

Thanks for the explanation. I hadn't realized that the REPL and DrRacket both arranged to do evaluation in the eventspace handler thread. I'll update the pull request.

Are the problems with GUI calls from non-eventspace threads documented somewhere? The only relevant section I found is Eventspaces and Threads, which did not contain as forceful a warning as you gave in this thread.

rmculpepper avatar May 30 '22 12:05 rmculpepper

Your right that is Eventspaces and Threads is the current documentation. Does the draft revision at https://github.com/mflatt/gui/commit/22c06cd4ff799959bdab1dc90bda0c9e18480d49 look closer to the right message?

mflatt avatar May 30 '22 15:05 mflatt