Browse Source

Fix code that could lead to a possible deadlock. (#1380)

* Fix code that could lead to a possible deadlock.

Drop implementations are not called until the end of a statement. The statement changed in this commit therefore took 4 read locks on a RwLock which can lead to problems if a write lock is requested between any of these read locks. The code looks like it would only hold one lock at a time but it does not drop any of the locks until after the arithmatic operations complete, which leads to this situation. See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=996079046184329f3a9df1cd19c87da8 to see this in action. The fix is to just take one lock and share it between the three calls to num_presses, letting it drop at the end of the scope.

* Fix code that may cause a deadlock in `MenuRoot::stationary_interaction`

The issue here is related to that in 9673b8f2a0 in that the lock is not dropped when it is expected.  Since the `RwLockReadGuard` produced by `ctx.input()` has a reference taken from it (one into `pointer`), the lock cannot be dropped until that reference is no longre valid, which is the end of the scope (in this case this function).  The following `ctx.input()` then attempts to aquire a second lock on the `RwLock`, creating the same situation that was found in the referenced commit.

This has been resolved by holding one lock on the input for the whole function.

* Reference this PR from comments in the code for future maintainers

* Add the change to the changelog

* Use full link to PR

* Use full link to PR

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
pull/1392/head
Zachary Kohnen 3 years ago
committed by GitHub
parent
commit
8bb381d50b
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      CHANGELOG.md
  2. 12
      egui/src/menu.rs
  3. 12
      egui/src/widgets/slider.rs

2
CHANGELOG.md

@ -17,7 +17,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
### Fixed 🐛
* Fixed ComboBoxes always being rendered left-aligned ([#1304](https://github.com/emilk/egui/pull/1304)).
* Fixed ui code that could lead to a deadlock ([#1380](https://github.com/emilk/egui/pull/1380))
## 0.17.0 - 2022-02-22 - Improved font selection and image handling

12
egui/src/menu.rs

@ -277,10 +277,10 @@ impl MenuRoot {
root: &mut MenuRootManager,
id: Id,
) -> MenuResponse {
let pointer = &response.ctx.input().pointer;
if (response.clicked() && root.is_menu_open(id))
|| response.ctx.input().key_pressed(Key::Escape)
{
// Lock the input once for the whole function call (see https://github.com/emilk/egui/pull/1380).
let input = response.ctx.input();
if (response.clicked() && root.is_menu_open(id)) || input.key_pressed(Key::Escape) {
// menu open and button clicked or esc pressed
return MenuResponse::Close;
} else if (response.clicked() && !root.is_menu_open(id))
@ -290,8 +290,8 @@ impl MenuRoot {
// or button hovered while other menu is open
let pos = response.rect.left_bottom();
return MenuResponse::Create(pos, id);
} else if pointer.any_pressed() && pointer.primary_down() {
if let Some(pos) = pointer.interact_pos() {
} else if input.pointer.any_pressed() && input.pointer.primary_down() {
if let Some(pos) = input.pointer.interact_pos() {
if let Some(root) = root.inner.as_mut() {
if root.id == id {
// pressed somewhere while this menu is open

12
egui/src/widgets/slider.rs

@ -455,10 +455,14 @@ impl<'a> Slider<'a> {
fn value_ui(&mut self, ui: &mut Ui, position_range: RangeInclusive<f32>) -> Response {
// If `DragValue` is controlled from the keyboard and `step` is defined, set speed to `step`
let change = ui.input().num_presses(Key::ArrowUp) as i32
+ ui.input().num_presses(Key::ArrowRight) as i32
- ui.input().num_presses(Key::ArrowDown) as i32
- ui.input().num_presses(Key::ArrowLeft) as i32;
let change = {
// Hold one lock rather than 4 (see https://github.com/emilk/egui/pull/1380).
let input = ui.input();
input.num_presses(Key::ArrowUp) as i32 + input.num_presses(Key::ArrowRight) as i32
- input.num_presses(Key::ArrowDown) as i32
- input.num_presses(Key::ArrowLeft) as i32
};
let speed = match self.step {
Some(step) if change != 0 => step,
_ => self.current_gradient(&position_range),

Loading…
Cancel
Save