Repository navigation
gtk: allocate Gtk memory with 'atomic-interior - #375
Merged
Merged
Conversation
Member
Author
|
This won't pass on CI until tomorrow, because it needs the FFI type added in racket/racket#5617. |
It's possible to call back into Racket during a Gtk call, via logging, signal handling, and other operations. If the Racket code triggers a collection, Gtk structs could move in the heap, leading to a crash. Avoid that by using `'atomic-interior` mode with all the data that Gtk can reach.
samth
force-pushed
the
gtk-atomic-interior
branch
from
October 20, 2026 17:43
016dbf4 to
4dd4cc3
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Resyntax analyzed 22 files in this pull request and has added suggestions.
| (define-gtk gtk_tree_view_get_visible_range (_fun _GtkWidget [sp : (_ptr o _pointer)] [ep : (_ptr o _pointer)] | ||
| (define-gtk gtk_tree_view_get_visible_range (_fun _GtkWidget [sp : (_ptr o _pointer atomic-interior)] [ep : (_ptr o _pointer atomic-interior)] | ||
| -> [ok? : _gboolean] | ||
| -> (values (if ok? sp #f) (if ok? ep #f)))) |
There was a problem hiding this comment.
if-else-false-to-and: This if expression can be refactored to an equivalent expression using and.
Suggested change
| -> (values (if ok? sp #f) (if ok? ep #f)))) | |
| -> (values (and ok? sp) (if ok? ep #f)))) |
Debugging details
Textual replacement
(line-replacement
#:new-lines
'#(" -> (values (and ok? sp) (if ok? ep #f))))")
#:original-lines
'#(" -> (values (if ok? sp #f) (if ok? ep #f))))")
#:start-line 89)Syntactic replacement
(syntax-replacement
#:introduction-scope #<procedure:...and/syntax-local.rkt:148:2>
#:new-syntax
#<syntax:/home/runner/.local/share/racket/9.4.0.2/pkgs/resyntax/default-recommendations/boolean-shortcuts.rkt:63:2 (and ok? sp)>
#:original-syntax
#<syntax:gui-lib/mred/private/wx/gtk/list-box.rkt:89:61 (if ok? sp #f)>
#:source
(file-source
#<path:/home/runner/work/gui/gui/gui-lib/mred/private/wx/gtk/list-box.rkt>)
#:uses-universal-tagged-syntax? #f)| (define-gtk gtk_tree_view_get_visible_range (_fun _GtkWidget [sp : (_ptr o _pointer)] [ep : (_ptr o _pointer)] | ||
| (define-gtk gtk_tree_view_get_visible_range (_fun _GtkWidget [sp : (_ptr o _pointer atomic-interior)] [ep : (_ptr o _pointer atomic-interior)] | ||
| -> [ok? : _gboolean] | ||
| -> (values (if ok? sp #f) (if ok? ep #f)))) |
There was a problem hiding this comment.
if-else-false-to-and: This if expression can be refactored to an equivalent expression using and.
Suggested change
| -> (values (if ok? sp #f) (if ok? ep #f)))) | |
| -> (values (if ok? sp #f) (and ok? ep)))) |
Debugging details
Textual replacement
(line-replacement
#:new-lines
'#(" -> (values (if ok? sp #f) (and ok? ep))))")
#:original-lines
'#(" -> (values (if ok? sp #f) (if ok? ep #f))))")
#:start-line 89)Syntactic replacement
(syntax-replacement
#:introduction-scope #<procedure:...and/syntax-local.rkt:148:2>
#:new-syntax
#<syntax:/home/runner/.local/share/racket/9.4.0.2/pkgs/resyntax/default-recommendations/boolean-shortcuts.rkt:63:2 (and ok? ep)>
#:original-syntax
#<syntax:gui-lib/mred/private/wx/gtk/list-box.rkt:89:76 (if ok? ep #f)>
#:source
(file-source
#<path:/home/runner/work/gui/gui/gui-lib/mred/private/wx/gtk/list-box.rkt>)
#:uses-universal-tagged-syntax? #f)
Comment on lines
+184
to
+190
| (for ([str (in-list types)] | ||
| [i (in-naturals)]) | ||
| (let ([t (ptr-add targets i _GtkTargetEntry)]) | ||
| (cpointer-push-tag! t 'GtkTargetEntry) | ||
| (set-GtkTargetEntry-target! t (ptr-add target-strings offset)) | ||
| (set-GtkTargetEntry-target! t str) | ||
| (set-GtkTargetEntry-flags! t 0) | ||
| (set-GtkTargetEntry-info! t i)) | ||
| (let ([bstr (string->bytes/utf-8 str)]) | ||
| (memcpy target-strings offset bstr 0 (bytes-length bstr)) | ||
| (let ([offset (+ offset (bytes-length bstr))]) | ||
| (ptr-set! (ptr-add target-strings offset) _byte 0) | ||
| (+ offset 1)))) | ||
| (set-GtkTargetEntry-info! t i))) |
There was a problem hiding this comment.
let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
Suggested change
| (for ([str (in-list types)] | |
| [i (in-naturals)]) | |
| (let ([t (ptr-add targets i _GtkTargetEntry)]) | |
| (cpointer-push-tag! t 'GtkTargetEntry) | |
| (set-GtkTargetEntry-target! t (ptr-add target-strings offset)) | |
| (set-GtkTargetEntry-target! t str) | |
| (set-GtkTargetEntry-flags! t 0) | |
| (set-GtkTargetEntry-info! t i)) | |
| (let ([bstr (string->bytes/utf-8 str)]) | |
| (memcpy target-strings offset bstr 0 (bytes-length bstr)) | |
| (let ([offset (+ offset (bytes-length bstr))]) | |
| (ptr-set! (ptr-add target-strings offset) _byte 0) | |
| (+ offset 1)))) | |
| (set-GtkTargetEntry-info! t i))) | |
| (for ([str (in-list types)] | |
| [i (in-naturals)]) | |
| (define t (ptr-add targets i _GtkTargetEntry)) | |
| (cpointer-push-tag! t 'GtkTargetEntry) | |
| (set-GtkTargetEntry-target! t str) | |
| (set-GtkTargetEntry-flags! t 0) | |
| (set-GtkTargetEntry-info! t i)) |
Debugging details
Textual replacement
(line-replacement
#:new-lines
'#(" (for ([str (in-list types)]"
" [i (in-naturals)])"
" (define t (ptr-add targets i _GtkTargetEntry))"
" (cpointer-push-tag! t 'GtkTargetEntry)"
" (set-GtkTargetEntry-target! t str)"
" (set-GtkTargetEntry-flags! t 0)"
" (set-GtkTargetEntry-info! t i))")
#:original-lines
'#(" (for ([str (in-list types)]"
" [i (in-naturals)])"
" (let ([t (ptr-add targets i _GtkTargetEntry)])"
" (cpointer-push-tag! t 'GtkTargetEntry)"
" (set-GtkTargetEntry-target! t str)"
" (set-GtkTargetEntry-flags! t 0)"
" (set-GtkTargetEntry-info! t i)))")
#:start-line 184)Syntactic replacement
(syntax-replacement
#:introduction-scope #<procedure:...and/syntax-local.rkt:148:2>
#:new-syntax
#<syntax:.../resyntax/base.rkt:215:28 (for ((str (in-list types)) (i (in-naturals))) (define t (ptr-add targets i _GtkTargetEntry)) (cpointer-push-tag! t (quote GtkTargetEntry)) (set-GtkTargetEntry-target! t str) (set-GtkTargetEntry-flags! t 0) (set-GtkTargetEntry-info! t i))>
#:original-syntax
#<syntax:gui-lib/mred/private/wx/gtk/clipboard.rkt:184:10 (for ((str (in-list types)) (i (in-naturals))) (let ((t (ptr-add targets i _GtkTargetEntry))) (cpointer-push-tag! t (quote GtkTargetEntry)) (set-GtkTargetEntry-target! t str) (set-GtkTargetEntry-flags! t 0) (set-GtkTargetEntry-info! t i)))>
#:source
(file-source
#<path:/home/runner/work/gui/gui/gui-lib/mred/private/wx/gtk/clipboard.rkt>)
#:uses-universal-tagged-syntax? #f)
Member
|
This looks right to me. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's possible to call back into Racket during a Gtk call, via logging, signal handling, and other operations. If the Racket code triggers a collection, Gtk structs could move in the heap, leading to a crash. Avoid that by using
'atomic-interiormode with all the data that Gtk can reach.