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 9673b8f 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>

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
 

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

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),