New API consideration: JoinableTaskContext.OnMainThreadBlocked about vs-threading HOT 6 OPEN

• @richardstanton , @AArnott

from vs-threading.

a special JTF factory, which does not PostToUnderlyingSynchronizationContext, so the inner task only runs, when the current task is blocking the main thread. It needs to chain clean-up logic correctly to prevent memory leaks.

What if that condition never happens? Tasks that never complete lead to async methods that never execute their finally statements, which would seem to be a problem.
What if we cancel or fault the returned Task if the JoinableTask completes before it becomes blocking?

from vs-threading.

that is why this OnMainThreadBlockedByCurrentTask returns a disposable, which can be disposed by the caller when it is no longer needed. In the implementation, this cancels the switching to UI thread, and essentially kill the inner task.

when it is implemented inside JTF, it can also happen when the context JTF task is completed just like your suggestion, so it would not lead memory leak if the user code doesn't dispose it correctly. I guess that would keep it safer than what we can do outside the JTF library.

from vs-threading.

Is this an accurate, super-high level summary:

You want to be able to keep the threadpool busy with solution load related work without the risk that it gets to the UI thread mid-load, but also without the risk that such a limitation creates deadlocks should solution load block on that very work?

If so, I can see how detecting that the main thread is now blocked on the work being a convenient way to allow 'background' work to proceed onto the main thread -- but it depends on solution open being overall a main-thread-blocking operation. Because if solution open ever stopped blocking the main thread, it seems to me your proposed algorithm here would lead to a solution load hang -- without causing a UI delay. Solution open might depend on a background task completing, yet nothing on the main thread is blocking on it.

Your opening problem statement also makes me wonder how you came to this design proposal. If the goal is to keep certain code from calling legacy IVs APIs before they are initialized, relying on main thread access to naturally prevent such calls from happening seems like a very brittle thing.

I'm guessing I'm just misunderstanding your proposal. Perhaps you can schedule a meeting to discuss?

from vs-threading.

the API is to allow a special callback when the current JoinableTask is blocking the UI thread, but is never called when the UI thread is idle.

it is designed for some scenarios, for example:

1, when a design time build is blocking the UI thread, raises the priority of it in the pending queue.

2, during the solution loading time, some low priority works are artificially blocked (like DT builds), in order to allow other work to run first. However, they need be unblocked if some features block UI thread to wait on that. Normal JTF work works until recently, when the solution loading is no longer blocking the UI thread, because SwitchToMainThread would be unblocked when the UI thread pumps messages.

we have already used this pattern on the project side, the reason to talk about this is that we found product code today is using SwitchToMainThread to yield until the solution is loaded, which is now broken after solution load becomes async.

I can arrange a meeting to discuss this.

from vs-threading.

It is not the exact problem we try to resolve here.

The problem in the product is that we found some code now run earlier than the old solution loading order after solution load becomes async. Before the solution load become async, some existing async code are blocked on SwitchToMainThreadAsync() until the solution is fully loaded, but now it gets a chance to run in the middle of loading a solution, because UI thread pumps messages.

That come in several different patterns:
1, we have code to use SwitchToMainThreadAsync() as a soft blocking point to delay running code until the solution is loaded for performance reasons (generally we want to delay lower priority work, instead of trying to do all as faster as possible during this high contention phase). but the waiting point still allows to pass when UI thread is blocked to wait the task.. A typical sample is that CPS holds design time build after solution is loaded, but if an extension accesses output groups (COM API) inside solution/project loaded event, it would allow build to start immediately to prevent a deadlock. (But that is still not desirable, and we want extensions to move away from doing that, so when this happens, it can send telemetry.)

The new API is largely designed that this pattern can easily to move to the new API, so it can wait and escape from the waiting point as soon as it is blocking the UI thread (and resume what it should do.)

2, on the other side, some code calls SwitchToMainThreadAsync() before calling COM APIs. However, once the code runs in the middle of solution loading time, it runs into many problems, because IVsSolution/IVsHierarchy is not yet in the consistent state in the middle of the async initialization. So those code could run into failures, because it could not find some projects, and some code tryto call IVsHierarchy before the project system finishes loading a project and returns from CreateProject/CreateProjectAsync. (For example, passing the IVsHierarchy to IVsSolution to get unique name, before IVsHierarchy is known to the solution.) So, basically, we need a better way to delay those code to a point that the solution COM object is ready to be used.

In this situation, the code actually cannot run correctly in the middle of loading a solution. When it is blocking a JTF.Run in the middle of loading a solution, it would be a deadlock bug. It could happen, because the code waits on it might get run too earlier as well, so the API is just to help to detect the problem, but the product would need further fixes when it happens.

Now, completely independent to this, we also have several pieces of code wants to detect works (evaluation/design time builds) blocking UI thread, so it can move the specific work to a high priority queue (in the throttle queue), so a part of this API is to have a consistent implementation, instead of doing it repeatedly (and easily gets it wrong). That is completely for performance reasons. (When the UI thread waits output group queued after other 100 projects in the build queue, it will complete in the end, just after a long delay.)

Of course, the API is not to prevent all type of dead locks. As you said, if the solution loading is waiting on a task, which awaits on solution loading to complete, and nothing blocks on the UI thread, it would never complete even without blocking the main thread.

I don't think it is a task library's responsibility to prevent this type of deadlock. The good part, it would not happen with current COM APIs, which blocks the UI thread and wait, so likely we will only see it, when more things move to async API. And hopefully, any issue pops up during the transition would be detected, and fixed.

This type of deadlock could happen inside the project itself, or due to extensions.

If a project system creates an async task, which must be completed to complete loading a project, but waits the solution to be loaded, it is a serious bug in the project side, and it usually will happen immediately, and be detected, and fixed. In CPS, the guideline is all project background loading tasks cannot depend on UI thread and any COM API. when it happens in the old world, it would break project background loading (as the task cannot run until CreateProject adds it to JTF dependency chain and unblocks it). I guess we should further enforce that rule.

But, your suggested scenario might happen when we move solution/project loaded event to async, and if an extension handing the event waits on an async API (like an async API to get output group), and inside the async API awaits solution load to complete, and yes, we will run into a state that nothing blocks on the UI thread, but solution load will never complete. I think the key point here is that waiting point we introduce here cannot include time to fire solution/project events. It is actually an point of time to indicate that solution/COM objects are in the state (instead of it is in the middle of async initialization time), which cannot be delayed by most VS extensions. And task blocked should be resumed before solution load completion event is fired. + @richardstanton to this, because he is working on that design

Is this an accurate, super-high level summary:

You want to be able to keep the threadpool busy with solution load related work without the risk that it gets to the UI thread mid-load, but also without the risk that such a limitation creates deadlocks should solution load block on that very work?

from vs-threading.