-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[browser][MT] Make ChangeLocalTimeZone thread-safe #103953
Conversation
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locally I cannot reproduce running in the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this is just hiding real underlying issue with emscripten.
It is posting async messages to UI thread from whatever thread exeuted the TZ change. And those messages probably come in random order.
Let's reconsider.
Let's revert this change, because it effectively hides the problem from the test. Let's rather create additional test which will change TZ from many different threads in threadpool. |
well, change on windows has an effect in the whole system, including registries, so if we do it concurrently I would assume a race and undefined expected result |
You are right, my bad. We should not try to make it work in parallel from multiple threads. We need to catch one (non-UI) thread doing it and the change being propagated too slowly. So that we can understand how it works in emscripten and what we need to synchronize in Mono. To prevent this "read after write hazard" or whatever it is. |
Do I understand it right that we drop the concept of improving test, revert the lock changes and now the goal is to pin debugger and investigate what is the reason for the failure? This failure was hit once per few weeks, any suggestions how to increase the probability of reproduction? |
I think so. Current code is how people write TZ code and it's broken only on MT WASM.
Yes, I don't expect that you would be able to use debugger.
Run it in a loop 1000 times. |
Thanks @pavel and @ilonatommy for clarification. |
Fixes #101166