[browser][MT] Log ManagedThreadId and NativeThreadId for known test failures#98291
[browser][MT] Log ManagedThreadId and NativeThreadId for known test failures#98291mkhamoyan merged 5 commits intodotnet:mainfrom
Conversation
mkhamoyan
added
arch-wasm
|
Tagging subscribers to 'arch-wasm': @lewing |
ghost
added
the
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| await tcs.Task; | ||
| }, cts.Token); | ||
|
|
||
| Console.WriteLine("ThreadingTimer: ManagedThreadId: " + Environment.CurrentManagedThreadId + " NativeThreadId: " + WebWorkerTestHelper.NativeThreadId); |
There was a problem hiding this comment.
WebWorkerTest.ThreadingTimer didn't hit at all in the log. So it would be also good to print timestamp before (line 367) and after (380)
Maybe we just need to move hit = true; before tcs.SetResult(); on line 374, because that's race conditon in the test.
There was a problem hiding this comment.
Updated this part accordingly.
| exception = ex; | ||
| } | ||
|
|
||
| Console.WriteLine("WaitAssertsOnJSInteropThreads: ManagedThreadId: " + Environment.CurrentManagedThreadId + " NativeThreadId: " + WebWorkerTestHelper.NativeThreadId); |
There was a problem hiding this comment.
would this tell us which and why Value is null ? #97914
There was a problem hiding this comment.
ok, we know which exception, we don't know why.
The "why" is likely because the call is executed on different thread than we expected.
There was a problem hiding this comment.
this could also be the "why"
This code is swallowing OperationCanceledException instead of passing it
There was a problem hiding this comment.
Added logs to validate if that is the case.
| { | ||
| CancellationTokenSource cts = new CancellationTokenSource(); | ||
| var promise = response.Content.ReadAsStringAsync(cts.Token); | ||
| Console.WriteLine("HttpClient_CancelInDifferentThread: ManagedThreadId: " + Environment.CurrentManagedThreadId + " NativeThreadId: " + WebWorkerTestHelper.NativeThreadId); |
There was a problem hiding this comment.
use ThrowsAnyAsync<OperationCanceledException> on line 81 may hide the problem.
The question is why we have 2 different code paths.
Please log stack traces in both cases, so that we could compare them.
There was a problem hiding this comment.
I kept ThrowsAsync<TaskCanceledException>, so issue will still happen but added logs to see stack traces for both TaskCanceledException and OperationCanceledException.
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
...ervices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.Http.cs
Show resolved
Hide resolved
| try { | ||
| mr.Wait(cts.Token); | ||
| } catch (OperationCanceledException) { /* ignore */ } | ||
| } catch (OperationCanceledException) { |
There was a problem hiding this comment.
it should not be swallowed I think. Because it's not really the expected outcome.
If it was expected outcome, the outer assert should process that case, rather than misleading null, which for this test means that something is broken for Main and WebWorker executors.
There was a problem hiding this comment.
Updated to not swallow OperationCanceledException.
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| mr.Wait(cts.Token); | ||
| } catch (OperationCanceledException) { /* ignore */ } | ||
| } catch (Exception ex) { | ||
| if (ex is OperationCanceledException) |
There was a problem hiding this comment.
Radek's solution here is bit better #98508 (comment)
There was a problem hiding this comment.
Removed these changes.
pavelsavara
left a comment
pavelsavara
left a comment
There was a problem hiding this comment.
Please observe the outcome and let us know what we learned from it. Thank you!
2 participants
Log ManagedThreadId and NativeThreadId for following test failures
#98101, #98216, #97914
Once test failures will be fixed, we will remove these logs.