Conversation
Fix and re-enable test that would catch this error
|
@jinujoseph @jasonmalinowski @heejaechang for review |
| var kind = ForegroundThreadDataInfo.CreateDefault(defaultKind); | ||
|
|
||
| return new ForegroundThreadData(Thread.CurrentThread, new SynchronizationContextTaskScheduler(SynchronizationContext.Current), kind); | ||
| return new ForegroundThreadData(Thread.CurrentThread, SynchronizationContext.Current == null ? TaskScheduler.Default : new SynchronizationContextTaskScheduler(SynchronizationContext.Current), kind); |
There was a problem hiding this comment.
Actually, should this be TaskScheduler.Current? I don't presume we're using this much in the /build case, but Default does mean we're always picking the thread pool. Current would mean it might be right (and might still be wrong, but whatever.)
There was a problem hiding this comment.
https://msdn.microsoft.com/en-us/library/system.threading.tasks.taskscheduler.current(v=vs.110).aspx
Remarks
When not called from within a task, Current will return the Default scheduler.
..
so in this case, it will be Default anyway?
There was a problem hiding this comment.
If we run this inside of an xunit unit test, then it might pick up the xunit scheduler or something which might be what we want there.
There was a problem hiding this comment.
Although I guess in that case SynchronizationContext.Current won't be null anyways. 🤷♂️
|
Approved via Link |
Customer scenario
Create a solution and from command line run:
devenv <solutionname>.sln /buildBugs this fixes
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/559223
fixes #18204
Workarounds, if any
build from IDE
Risk
None, just added a null check on a method call
Performance impact
None
Is this a regression from a previous update?
Root cause analysis
We are re-enabling this test: #18204 to catch this error.
How was the bug found?
Internal - VS team
Test documentation updated?
n/a