Skip to content
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

Fix disposal of faulted WCF client channels #322

Merged

Conversation

jberezanski
Copy link
Contributor

Fix WCF client channel cleanup in WcfFacility, so that WCF channels are appropriately closed/aborted before being disposed. Otherwise, broken channels may cause exceptions to be thrown during container or scope disposal.

Fixes GH-71 and GH-104.

The disposal order in WcfClientActivator matters: WcfChannelHolder knows how
to properly close/abort broken channels, while the WCF IDisposable
implementation will throw an exception. Therefore, WcfChannelHolder.Dispose()
needs to be called first, before the cleanup logic in
AbstractComponentActivator invokes the WCF IDisposable implementation (via
the DisposalConcern).

The invocation order was originally correct, but it got inverted in 0c30043
"Improved concurrency during WCF channel failover".

Restore the proper cleanup order, so that WCF channels are appropriately
closed/aborted before being disposed.

Fixes castleprojectGH-71 and castleprojectGH-104.
@jberezanski
Copy link
Contributor Author

I'm guessing the AppVeyor build failure is not related to my changes:

NUnit.Engine.NUnitEngineException: The path specified in --result src\Castle.Windsor.Tests\bin\Release\net45\TestResult_Windsor.xml could not be written to ---> System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\projects\windsor\src\Castle.Windsor.Tests\bin\Release\net45\TestResult_Windsor.xml'.

All tests passed when I invoked them locally (on .NET 4.6.2).

@jonorossi
Copy link
Member

Sorry for the delay, yes master is broken because of #321.

The change looks good, I'll take another look after getting master sorted this weekend. If you get a chance to see this before I merge it could you update the changelog to include your fix.

@jonorossi jonorossi merged commit 3df6e60 into castleproject:master Sep 11, 2017
@jonorossi
Copy link
Member

Merged, thanks.

@jberezanski
Copy link
Contributor Author

Thank you! Sorry, I've been out of town so I did not see your request for changelog update until now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Channels are always Disposed even if they are in Faulted state [FACILITIES-167]
2 participants