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

The application freezes while resizing the terminal on MacOS #1683

Closed
kacperpikacz opened this issue Apr 28, 2022 · 20 comments · Fixed by #2668
Closed

The application freezes while resizing the terminal on MacOS #1683

kacperpikacz opened this issue Apr 28, 2022 · 20 comments · Fixed by #2668

Comments

@kacperpikacz
Copy link

Describe the bug
The whole app freeze If I resize the terminal while Application fetching the current time from internet.

To Reproduce
https://www.toptal.com/developers/hastebin/iruwokoxuw.csharp

Desktop:

  • MacOS

Additional context
I don't know if it's a bug or I'm doing something wrong. But the same issue you will get in Threading scenario from UICatalog if you run few tasks and start playing with terminal size by mouse.

@tig tig added the bug label Apr 28, 2022
@tznind
Copy link
Collaborator

tznind commented Apr 28, 2022

I updated the code a bit as I couldn't get yours to work straight off. Here it is:

https://www.toptal.com/developers/hastebin/xizitagije.csharp

I added a couple of test entries to the list and initialized the collection before the list view. I am not able to reproduce the hanging issue and can resize on both Windows and Linux (including with UseSystemConsole) so this might be a MacOS only issue?

@kacperpikacz
Copy link
Author

You should start playing with terminal size just after running an app.

https://media.giphy.com/media/9ap4iFOBViDFdcgErl/giphy.gif

@tznind
Copy link
Collaborator

tznind commented Apr 28, 2022

I'm guessing this must be a MacOS issue. I don't have a MacOS device to test with and as you can see below the behavior isn't replicated on Linux (or Windows) - I tested with Powershell, Visual Studio Terminal, Linux Terminology and Terminator. Maybe one of the other maintainers will be able to help more, sorry.

resizing

Just to confirm that you only see this behavior when resizing?

@kacperpikacz
Copy link
Author

kacperpikacz commented Apr 28, 2022

Yes, only while resizing.

You change terminal dimensions after the InternetTime.GetCurrentTime() finished. Start to play with terminal size immediately after dotnet run.

I checked now, Windows don't have this issue.

@tznind
Copy link
Collaborator

tznind commented Apr 30, 2022

Probably a long shot @kacperpikacz but could you see if my PR changes anything for you with this issue #1684 . It fixes some threading issues to do with timeouts so might help (or it could be entirely unrelated to that).

@kacperpikacz
Copy link
Author

@tznind just checked your modified MainLoop class but it doesn't fix issue in this case.

Application.UseSystemConsole = true will fix this issue. So the problem is somewhere inside ConsoleDriver

@0x6f6f66
Copy link

Can confirm that I'm having a similar issue on MacOS terminal.

Screen.Recording.2022-05-22.at.14.53.35.mov

@tig tig added the MacOS label Aug 4, 2022
@tig tig changed the title The application freezes while resizing the terminal. The application freezes while resizing the terminal on MacOS Aug 4, 2022
@tig
Copy link
Collaborator

tig commented Aug 4, 2022

Does this still repo?

@tig
Copy link
Collaborator

tig commented Sep 16, 2022

Closing as not-repo

@tig tig closed this as completed Sep 16, 2022
@kacperpikacz
Copy link
Author

kacperpikacz commented May 22, 2023

@tig The issue is still there.

Untitled.mp4

@tig tig reopened this May 22, 2023
@kacperpikacz
Copy link
Author

kacperpikacz commented May 23, 2023

I've checked versions 1.6.4, 1.5, 1.4 from NuGet now and I can confirm that the issue with my code example doesn't exist there (not sure about Threading Scenario from UICatalog). If I update version to any above 1.6.4 the issue is back again.

✅ Version 1.6.4 - Example with Label (No issue with ListView also)
https://hastebin.com/share/cevigenoze.csharp
Untitled

⁉️ Version 1.11.2 - Example with ListView (Same issue will be with Label)
https://hastebin.com/share/ozukenatof.csharp
Untitled-2

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2023

@kacperpikacz it seems that you are dealing with some threading job and updating the screen without using Application.MainLoop.Invoke (() => do screen update here in the UI thread).

@kacperpikacz
Copy link
Author

@kacperpikacz it seems that you are dealing with some threading job and updating the screen without using Application.MainLoop.Invoke (() => do screen update here in the UI thread).

It's not about updating screen.

https://hastebin.com/share/ucehihumoy.csharp

Untitled.mov

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2023

You need to update the TextField like this. Here you don't need to call Application.MainLoop.Invoke because the AddTimeout already handles that. You also don't need to call SetNeedsDisplay because the Text property already do that.

		static Func<MainLoop, bool> fetchTime = (loop) => {
			UpdateTimeAsync ();
			MainField.Text = Clock.unixtime.ToString ();
			//MainField.SetNeedsDisplay ();

			return true;
		};

@tznind
Copy link
Collaborator

tznind commented May 23, 2023

@kacperpikacz do you get this issue when you run the demo application (UICatalog) in this repository?

If not then it is probably to do with cross thread access as @BDisp says.

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2023

@kacperpikacz I confirm that the Threading scenario hangs after a while. This only happens on macOS, not on Windows and Linux. I'll try to find the cause but without promises.

@BDisp
Copy link
Collaborator

BDisp commented May 23, 2023

@kacperpikacz please test my PR #2667 with the fix. Thanks.

BDisp added a commit to BDisp/Terminal.Gui that referenced this issue May 23, 2023
@tig tig closed this as completed in #2668 May 24, 2023
tig pushed a commit that referenced this issue May 24, 2023
* Fixes #1683. The application freezes while resizing the terminal on MacOS

* Remove unused variable.
tig pushed a commit that referenced this issue May 24, 2023
…acOS (#2667)

* Fixes #1683. The application freezes while resizing the terminal on MacOS

* Remove unused variable.
@kacperpikacz
Copy link
Author

kacperpikacz commented May 24, 2023

You need to update the TextField like this. Here you don't need to call Application.MainLoop.Invoke because the AddTimeout already handles that. You also don't need to call SetNeedsDisplay because the Text property already do that.

		static Func<MainLoop, bool> fetchTime = (loop) => {
			UpdateTimeAsync ();
			MainField.Text = Clock.unixtime.ToString ();
			//MainField.SetNeedsDisplay ();

			return true;
		};

I appreciate your recent fix @BDisp but as you can see in the code I'm not trying to update the TextField in any way. But application still will freeze because of UpdateTimeAsync()

I can confirm that your fix is working with threading scenario from UICatalog but not fixes issue what you can see below.

When you replace TextField with Label, there is no issue with resizing & UpdateTimeAsync method fire every 500 ms.

Code
https://hastebin.com/share/ucehihumoy.csharp

My mistake, I added the reference to the wrong project. I apologize for the misunderstanding. Recent commit fixes the problem.

Untitled.mov

@tig
Copy link
Collaborator

tig commented May 24, 2023

@kacperpikacz your comment above is confusing. Is this fixed now in develop or not? Thanks!

@kacperpikacz
Copy link
Author

@tig Sorry for the confusion. It's fixed now in develop.

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

Successfully merging a pull request may close this issue.

5 participants