-
Notifications
You must be signed in to change notification settings - Fork 703
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
v2win/v2net: UICatalog full benchmark suite crashes in Dialogs scenario #3941
Comments
I can't seem to get trace logs out @tig , I've tried with I do get the normal logs though... also tried capital V for verbose but no joy 😕 |
Fix is private static ILogger CreateLogger ()
{
// Configure Serilog to write logs to a file
+ _logLevelSwitch.MinimumLevel = Enum.Parse<LogEventLevel> (_options.DebugLogLevel);
Log.Logger = new LoggerConfiguration ()
Also /// <summary>
/// Specifies the meaning and relative importance of a log event.
/// </summary>
public enum LogEventLevel
{
/// <summary>
/// Anything and everything you might want to know about
/// a running block of code.
/// </summary>
Verbose,
/// <summary>
/// Internal system events that aren't necessarily
/// observable from the outside.
/// </summary>
Debug,
/// <summary>
/// The lifeblood of operational intelligence - things
/// happen.
/// </summary>
Information,
/// <summary>
/// Service is degraded or endangered.
/// </summary>
Warning,
/// <summary>
/// Functionality is unavailable, invariants are broken
/// or data is lost.
/// </summary>
Error,
/// <summary>
/// If you have a pager, it goes off when one of these
/// occurs.
/// </summary>
Fatal
} |
(I just edited the above). |
Grok says:
Which naming should we use? We should be consistent. |
Ah this is because UICatalog is implementing Serilog while the Terminal.Gui logger is Since the calls in core library are LogLevel we should probably use that for the command line but that means changing it into the Serilog level for the actual logger. |
I am struggling to understand this stack overflow, it seems to be tied up in the RaiseAccepting logic. But it does not seem to repro when using the UI directly and trying to press the coresponding buttons. Its like the Accepting is looking at the super view to compute IsDefault which is then running a new iteration and... unclear. I wonder if this is why we have sporadic test failures in CI, there is
|
Issue is probably in here: /// <summary>
/// Called when the user is accepting the state of the View and the <see cref="Command.Accept"/> has been invoked. Calls <see cref="OnAccepting"/> which can be cancelled; if not cancelled raises <see cref="Accepting"/>.
/// event. The default <see cref="Command.Accept"/> handler calls this method.
/// </summary>
/// <remarks>
/// <para>
/// The <see cref="Accepting"/> event should be raised after the state of the View has changed (after <see cref="Selecting"/> is raised).
/// </para>
/// <para>
/// If the Accepting event is not handled, <see cref="Command.Accept"/> will be invoked on the SuperView, enabling default Accept behavior.
/// </para>
/// <para>
/// If a peer-View raises the Accepting event and the event is not cancelled, the <see cref="Command.Accept"/> will be invoked on the
/// first Button in the SuperView that has <see cref="Button.IsDefault"/> set to <see langword="true"/>.
/// </para>
/// </remarks>
/// <returns>
/// <see langword="null"/> if no event was raised; input processing should continue.
/// <see langword="false"/> if the event was raised and was not handled (or cancelled); input processing should continue.
/// <see langword="true"/> if the event was raised and handled (or cancelled); input processing should stop.
/// </returns>
protected bool? RaiseAccepting (ICommandContext? ctx)
{
CommandEventArgs args = new () { Context = ctx };
// Best practice is to invoke the virtual method first.
// This allows derived classes to handle the event and potentially cancel it.
args.Cancel = OnAccepting (args) || args.Cancel;
if (!args.Cancel)
{
// If the event is not canceled by the virtual method, raise the event to notify any external subscribers.
Accepting?.Invoke (this, args);
}
// Accept is a special case where if the event is not canceled, the event is
// - Invoked on any peer-View with IsDefault == true
// - bubbled up the SuperView hierarchy.
if (!args.Cancel)
{
// If there's an IsDefault peer view in Subviews, try it
var isDefaultView = SuperView?.Subviews.FirstOrDefault (v => v is Button { IsDefault: true });
if (isDefaultView != this && isDefaultView is Button { IsDefault: true } button)
{
bool? handled = isDefaultView.InvokeCommand<KeyBinding> (Command.Accept, new ([Command.Accept], null, this));
if (handled == true)
{
return true;
}
}
if (SuperView is { })
{
return SuperView?.InvokeCommand<KeyBinding> (Command.Accept, new ([Command.Accept], null, this)) is true;
}
}
return Accepting is null ? null : args.Cancel;
} |
I think the Enter key is being raised over and over. I have a hypothesis. There is a TimedEvent in the stack. If the timed event raises Enter which calls Run. Then run does an iteration and finds the same timed event and triggers another Run. Because you never get out of the timed event callback it never gets removed or it's run time changed. Solution would be to prevent double entry into the same timed event callback |
This? private void OnApplicationNotifyNewRunState (object? sender, RunStateEventArgs e)
{
SubscribeAllSubviews (Application.Top!);
_currentDemoKey = 0;
_demoKeys = GetDemoKeyStrokes ();
Application.AddTimeout (
new TimeSpan (0, 0, 0, 0, BENCHMARK_KEY_PACING),
() =>
{
if (_currentDemoKey >= _demoKeys.Count)
{
return false;
}
Application.RaiseKeyDownEvent (_demoKeys [_currentDemoKey++]);
return true;
});
return;
// Get a list of all subviews under Application.Top (and their subviews, etc.)
// and subscribe to their DrawComplete event
void SubscribeAllSubviews (View view)
{
view.DrawComplete += (s, a) => BenchmarkResults.DrawCompleteCount++;
view.SubviewsLaidOut += (s, a) => BenchmarkResults.LaidOutCount++;
foreach (View subview in view.Subviews)
{
SubscribeAllSubviews (subview);
}
}
} |
Yep. -private const int BENCHMARK_KEY_PACING = 1; // Must be non-zero
+private const int BENCHMARK_KEY_PACING = 100; // Must be non-zero After \UICatalog\bin\Debug\net8.0\UICatalog.exe --driver v2win --benchmark Dialogs {
"Scenario": "Dialogs",
"Duration": "00:00:02.8324626",
"IterationCount": 0,
"ClearedContentCount": 17,
"RefreshedCount": 0,
"UpdatedCount": 0,
"DrawCompleteCount": 398,
"LaidOutCount": 34
} |
But I need to make this more rebust because that's stoopid. Not sure how. |
Not sure I understand how this is triggering infinite loop... the callback has Ah right, it's because the dialog is a popup which is a new run state. And when it closes its probably also a new run state maybe. This code needs to check to see if the run state being entered is a Scenario maybe? And that it is the first time entering that Scenario. |
Describe the bug
When running the full benchmark suite with either the v2win or v2net driver the Dialogs scenario crashes. Running the Dialogs scenario alone does not crash most of the time but it is also prone to crash.
The Dialogs scenario seems to be stuck for a while before crashing. Sometimes instead of crashing it is just stuck there at least longer than my patience persists.
To Reproduce
Choose one of the commands and run it in Release build:
Expected behavior
The benchmark suite finishes without crashes.
Screenshots
Running the full benchmark suite with both v2 drivers in VS the debug output shows a StackOverflowException when the benchmark crashes after message flood. Of course no stack trace is available when stack overflows.
After flooding for a while...
Memory is flooded rapidly at the time it happens.
Desktop
The text was updated successfully, but these errors were encountered: