Fix OnOpen/OnBeginOpen issues with custom channels #1571
Conversation
roncain
added
PR DO NOT MERGE
| @@ -798,12 +903,10 @@ internal protected virtual Task OnCloseAsync(TimeSpan timeout) | |||
|
|
|||
| internal protected virtual Task OnOpenAsync(TimeSpan timeout) | |||
There was a problem hiding this comment.
Reviewers: doing a code coverage run reveals this and the virtual OnCloseAsync here in the base class now appears to be dead code. This is consistent with all WCF product code overriding this method, plus the fact the way we call external types doesn't use the code path that gets here. I'm contemplating a hard assert here that we should never get here, and then remove the body of these methods. Thoughts?
| @@ -830,7 +933,7 @@ internal protected virtual Task OnOpenAsync(TimeSpan timeout) | |||
| } | |||
|
|
|||
| // Calls the given Action asynchronously. | |||
| private async Task CallActionAsync<TArg>(Action<TArg> action, TArg argument) | |||
| internal static async Task CallActionAsync<TArg>(Action<TArg> action, TArg argument) | |||
There was a problem hiding this comment.
Reviewers: I'm going to move this to TaskHelpers because it has no dependencies on CommunicationObject
|
Ping... any comments? From my perspective, this PR is complete. I'll squash out extra commits and rebase. /cc: @zhenlan @iamjasonp @mconnew |
|
In the cursory review I've done, I don't see anything that pops out... will take a closer look as time permits |
Fixes the product issue by propagating knowledge of sync and async open/close requests down the channel stack so they stay sync or async based on the original call. Created mocks and unit tests for custom channels Fixes dotnet#1544
roncain
force-pushed
the
custom_channel_fix
branch
from
4fd6e8c to
96e0c4d
Compare
|
Ping -- any comments? I rebased and squashed. No changes have been made for the past 10 days. Reminder: without this fix, the custom channel layer is broken. I have tested manually against NET Native, and of course all tests pass, including the ones added by this PR. |
|
|
|
Thanks @mconnew |
Fixes the issue where OnOpen/OnBeginOpen and
OnClose/OnBeginClose were incorrectly called when
using custom channels.
Fixes the product issue by propagating knowledge of
sync and async open/close requests down the channel
stack so they stay sync or async based on the original
call.
Created mocks and unit tests for custom channels
READY FOR REVIEW NOW
Fixes #1544