book: add "Don't keep locks while emitting signals" section#2301
Open
mvanhorn wants to merge 1 commit into
Open
book: add "Don't keep locks while emitting signals" section#2301mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
gtk-rs#392 asked for a small section in the signals chapter explaining why RefCell/Mutex must not be held across signal emission, with a self-contained example that demonstrates the problem and the fix. Adds a TrackedButton listing under book/listings/g_object_signals/3/ with a String property in a RefCell. ButtonImpl::clicked clones the String out of the RefCell first, drops the borrow, and only then calls emit_by_name. The handler in main.rs deliberately calls back into the button via set_text from inside text-changed, which only works because the borrow was dropped before emission - a direct demonstration of the re-entry the section warns about. Wires the listing into book/listings/Cargo.toml as g_object_signals_3 and adds a 'Don't keep locks while emitting signals' section in book/src/g_object_signals.md just before the closing summary. The example follows the maintainer suggestion to use a String-typed property as the easiest way to surface the issue (sdroege's comment on the issue thread). Closes gtk-rs#392
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
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.
Closes #392.
Summary
#392 asked for a small section in the signals chapter explaining why a
RefCellborrow (or aMutexguard) must not be held across signalemission, with a self-contained example that demonstrates the problem
and the fix. The maintainer's suggestion in the thread was to use a
String-typed property as the easiest way to surface the issue.
What's in the listing
book/listings/g_object_signals/3/:tracked_button/imp.rs- a custom button with atext: RefCell<String>property.ButtonImpl::clickedclones the Stringout of the RefCell first, drops the borrow, and only then calls
emit_by_name::<()>("text-changed", &[&snapshot]). AnANCHOR: clicked_goodblock delimits the part the chapter pulls invia
{{#rustdoc_include ...}}.tracked_button/mod.rs- the public wrapper, mirroring the listing-2shape exactly.
main.rs- the handler intentionally calls back into the button bycalling
set_textfrom insidetext-changed. This is the load-bearing detail: it only works because
clickeddropped the borrowbefore emitting. If the borrow had survived, the inner
set_textwould have aborted at
BorrowMutError. AnANCHOR: handlerblockdelimits the embedded snippet for the chapter.
What's in the chapter
book/src/g_object_signals.mdgets a "Don't keep locks while emittingsignals" section just before the closing summary. It states the rule
once, embeds the
clicked_goodsnippet, embeds the handler, andexplains why the fact that the handler doesn't panic is the actual
demonstration of the rule. It also notes that property notifications go
through the same dispatch path, so the same rule applies to
notify.Wiring
book/listings/Cargo.tomlgets ag_object_signals_3[[bin]]entrybetween the existing
g_object_signals_2andg_object_subclassing_1,matching the existing format.
Verification
mod.rswrappingimp,imp.rsderivingProperties+glib::object_subclass+ signalsvia
OnceLock<Vec<Signal>>,glib::derived_propertiesonObjectImpl).cargo checklocally because GTK 4 system libraries(
gtk4.pc,pkg-config) aren't available on this machine; I trackedthe existing patterns line-for-line against
listings/g_object_signals/2/and
book/listings/Cargo.tomlso the wiring matches what the rest ofthe book does.
{{#rustdoc_include ../listings/...}}mechanism the rest of the chapter already uses,with named anchors (
clicked_good,handler) that match theexisting
object_impl/button_impl/signal_handlingstyle.If anything fails to compile under GTK 4, happy to amend - the example
is small enough to iterate quickly.
Out of scope
bad pattern in prose and shows the good pattern in code, which keeps
the listing buildable as part of
g_object_signals_3and avoidscluttering the book's
cargo checkwith an example that's expectedto panic.
g_object_signals_1/_2listings.