felixfbecker / PSKubectl

kubectl with the power of the object pipeline
MIT License
61 stars 9 forks source link

Use AsyncCmdlet #1

Closed felixfbecker closed 6 years ago

felixfbecker commented 6 years ago

@tintoy I followed your suggestion of using AsyncCmdlet and it seems to work fine. Before, I could not cancel Get-KubeLog -Follow with ctrl+C, but now I can. However, it always throws this exception that crashes the PowerShell process:

^C

An error has occurred that was not properly handled. Additional information is shown below. The PowerShell process will exit.

Unhandled Exception: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'ThreadAffinitiveSynchronizationContext'.
   at Kubectl.ThreadAffinitiveSynchronizationContext.CheckDisposed() in /Users/felix/src/github.com/felixfbecker/PSKubectl/src/ThreadAffinitiveSynchronizationContext.cs:line 48
   at Kubectl.ThreadAffinitiveSynchronizationContext.Post(SendOrPostCallback callback, Object callbackState) in /Users/felix/src/github.com/felixfbecker/PSKubectl/src/ThreadAffinitiveSynchronizationContext.cs:line 98
   at System.Threading.Tasks.AwaitTaskContinuation.RunCallback(ContextCallback callback, Object state, Task& currentTask)
--- End of stack trace from previous location where exception was thrown ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location where exception was thrown ---
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

Any idea why?

tintoy commented 6 years ago

Hmm, let me have a look at the code for that cmdlet - I think you may need to take the call to CheckDisposed out of the Post method (and simply return if disposed).

tintoy commented 6 years ago

At first glance, it looks like the cancellation token is not being honoured, but it might simply be a race condition...

tintoy commented 6 years ago

What happens if you use observable.Subscribe instead of ForEachAsync?

felixfbecker commented 6 years ago

Aren't CancellationTokens best-effort? Since the callee can always be busy (CPU-blocked) and might still write a result before checking the token again, it can never be guaranteed that/when it is honored

tintoy commented 6 years ago

Ah, it might help if the client was calling ConfigureAwait(false). I'll see what I can do there :)

tintoy commented 6 years ago

Give me 5 mins

tintoy commented 6 years ago

See if this build helps?

Changes here: https://github.com/tintoy/dotnet-kube-client/commit/e27f178d3d17131ff091e36798f3237c26e0e119

https://travis-ci.org/tintoy/dotnet-kube-client/builds/418013614

felixfbecker commented 6 years ago

Hm, with 2.1.4-unstable0009 actually is worse. Before, the logs would get streamed correctly, the error would only get thrown when cancelling with Ctrl+C. Now, just invoking Get-KubeLog -Follow throws

System.Management.Automation.PSInvalidOperationException: The WriteObject and WriteError methods cannot be called from outside the overrides of the BeginProcessing, ProcessRecord, and EndProcessing methods, and they can only be called from within the same thread. Validate that the cmdlet makes these calls correctly, or contact Microsoft Customer Support Services.
   at System.Management.Automation.MshCommandRuntime.ThrowIfWriteNotPermitted(Boolean needsToWriteToPipeline)
   at System.Management.Automation.MshCommandRuntime.DoWriteObject(Object sendToPipeline)
   at System.Management.Automation.MshCommandRuntime.WriteObject(Object sendToPipeline)
   at System.Management.Automation.Cmdlet.WriteObject(Object sendToPipeline)
   at System.Reactive.Linq.QueryLanguage.<>c__DisplayClass408_1`1.<ForEachAsync_>b__2(TSource x)
--- End of stack trace from previous location where exception was thrown ---
   at Kubectl.GetKubeLogCmdlet.ProcessRecordAsync(CancellationToken cancellationToken) in /Users/felix/src/github.com/felixfbecker/PSKubectl/src/GetKubeLogCmdlet.cs:line 40
   at Kubectl.ThreadAffinitiveSynchronizationContext.RunSynchronized(Func`1 asyncOperation) in /Users/felix/src/github.com/felixfbecker/PSKubectl/src/ThreadAffinitiveSynchronizationContext.cs:line 151
   at Kubectl.AsyncCmdlet.ProcessRecord() in /Users/felix/src/github.com/felixfbecker/PSKubectl/src/AsyncCmdlet.cs:line 147
   at System.Management.Automation.Cmdlet.DoProcessRecord()
   at System.Management.Automation.CommandProcessor.ProcessRecord()

I guess the ConfigureAwait(false) caused the WriteObject() to not be called in the same thread anymore.

tintoy commented 6 years ago

Hmm - ok, I see what you mean, let me undo that last change.

tintoy commented 6 years ago

Can you try using logs.ObserveOn(SynchronizationContext.Current).ForEachAsync instead?

tintoy commented 6 years ago

(technically more correct, from memory)

felixfbecker commented 6 years ago

To be honest, I don't fully understand how the whole SynchronistationContext thing works, but I read that ConfigureAwait(false) is best practice for libraries.

logs.ObserveOn(SynchronizationContext.Current).ForEachAsync() works perfectly, thanks! Will remember that when dealing with Observables.

tintoy commented 6 years ago

Observables are tricky :)

It's to do with where continuations are run I think, every await is like another callback that may run either on any thread (i.e. don't care) or a specific thread (i.e. SynchonizationContext.Current.Post).

tintoy commented 6 years ago

ObserveOn causes subscriber OnXXX callbacks to be scheduled using the SynchonizationContext.Current (or an IScheduler).