Skip to content

Fix Console methods being split across threads #1622

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

Closed
wants to merge 6 commits into from
Closed

Fix Console methods being split across threads #1622

wants to merge 6 commits into from

Conversation

Nik300
Copy link
Contributor

@Nik300 Nik300 commented Jan 4, 2021

Screenshot 2021-01-05 000253

This should completely fix the bug we had with Console.ReadLine and threads ;)

@Nik300 Nik300 changed the title Fix Console.ReadLine being split across threads Fix Console methods being split across threads Jan 5, 2021
Nik300 added 2 commits January 6, 2021 01:32
Removing LockSystem that we don't need anymore
Modifying Console.ReadLine and Write to use Mutex instead of LockSystem
@Nik300 Nik300 requested a review from valentinbreiz January 7, 2021 08:49

}

private static List<char> chars = new List<char>(32);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move chars outside the ReadLine function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because if we put them into the function, every thread would have to access its own instance of characters and this would just waste memory and cause errors when threads that don't require "keyboard read" are forced to add to that list(that for them doesn't exist) a character.

@@ -601,17 +608,21 @@ public static void Write(bool aBool)
/* Correct behaviour printing null should not throw NRE or do nothing but should print an empty string */
public static void Write(object value) => Write((value ?? String.Empty));

private static Cosmos.System.Console xConsole = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

private static Encoding ConsoleOutputEncoding = Encoding.ASCII;
private static Encoding ConsoleOutputEncoding = Encoding.ASCII;
private static Core.Processing.Mutex mConsoleGateRead = new Core.Processing.Mutex();
private static Core.Processing.Mutex mConsoleGateWrite = new Core.Processing.Mutex();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please specify Cosmos.Core.Processing with an using rather than this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. will fix in a moment

@github-actions
Copy link

Stale pull request message

@valentinbreiz
Copy link
Member

Closing this for same reasons as #1921

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

Successfully merging this pull request may close these issues.

3 participants