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

fix sandbox GUI event handling #4272

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rmculpepper
Copy link
Collaborator

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.

@mflatt
Copy link
Member

mflatt commented May 29, 2022

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.

@rmculpepper
Copy link
Collaborator Author

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.

@mflatt
Copy link
Member

mflatt commented May 29, 2022

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.

@rmculpepper
Copy link
Collaborator Author

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.

mflatt added a commit to mflatt/gui that referenced this pull request May 30, 2022
@mflatt
Copy link
Member

mflatt commented May 30, 2022

Your right that is Eventspaces and Threads is the current documentation. Does the draft revision at mflatt/gui@22c06cd look closer to the right message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants