You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4net-dev@logging.apache.org by ahouben <gi...@git.apache.org> on 2015/03/05 02:53:11 UTC
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
GitHub user ahouben opened a pull request:
https://github.com/apache/log4net/pull/12
Fixes for the LogicalThreadContext
The current implementation of `LogicalThreadContext` does not flow correctly through `async/await` calls as described in http://stackoverflow.com/questions/23316743/log4net-logicalthreadcontext-not-working-as-expected.
This fix honors the shallow-copy-on-write behavior and immutable requirements when using `CallContext.Logical{Get,Set}Data`. Please see the accompanied `LogicalThreadContextTest` which is a copy of `ThreadContextTest` but adds `TestLogicalThreadPropertiesPatternAsyncAwait` and `TestLogicalThreadStackPatternAsyncAwait` to demonstrate the fix.
Thanks to @stephen-cleary for all the good information on the inner workings of the async/await mechanism all over the internet.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahouben/log4net feature/async
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/log4net/pull/12.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12
----
commit 72673fc46aa8abe4fd21018104352b143b7cce8d
Author: Alexander Houben <ah...@greenliff.com>
Date: 2015-03-05T01:17:28Z
Fix for the LogicalThreadContext{Stack,Properties}
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-87385635
I had to tweak things a little to make it build on Mono and it looks as if I had to do a little bit more for .NET 2.0, these will be follow up commits.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/log4net/pull/12
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-77686089
I'm fine with the first batch of changes you propose but don't think we should thrown an exception on .NET 4.0. I've opened [LOG4NET-455](https://issues.apache.org/jira/browse/LOG4NET-455) for tracking this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by ahouben <gi...@git.apache.org>.
Github user ahouben commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-77960511
The PR was updated to reflect the aforementioned changes:
* Create a new top level src/log4net/log4net.vs2012.sln derived from log4net.vs2010.sln with the following changes:
* targets .NET 4.5
* includes exactly the same sources as log4net.vs2010.sln
* defines an additional conditional compilation symbol FRAMEWORK_4_5_OR_ABOVE which is used to conditionally execute the new unit tests in the patch (Task.WhenAll used in the tests does not exist prior to .NET 4.5)
* revert log4net.[Tests.]vs2010.sln to target .NET 4.0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-77343333
FWIU the code should be compilable for .NET 4.0 but the changes will only fix the problem on 4.5 (and there is no fix for 4.0).
We'll likely need additional constants and conditionally compile the new unit tests on 4.5 only, which may become tricky since NAnt doesn't support .NET 4.5 as a target framework, yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by maxcherednik <gi...@git.apache.org>.
Github user maxcherednik commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-125192476
Hi,
I was playing around with this fix recently and came across some strange behavior.
Imagine main method:
using (LogicalThreadContext.Stacks["NDC"].Push("Start"))
{
// init all the stuff here
_timer.TimerElapsed += TimerOnTimerElapsed;
}
An that would be it.
Then I on the timer tick event I have another context:
using (LogicalThreadContext.Stacks["NDC"].Push("timer"))
{
Logger.Info("I am doing something here. Show me the context");
}
As a result expect to see the context of the timer like this: "timer",
instead, I see: "Start timer"
Is this correct behavior?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by ahouben <gi...@git.apache.org>.
Github user ahouben commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-77357942
Would it make sense to update the patch with the following modifications then:
· Create a new top level src/log4net/log4net.vs2012.sln derived from log4net.vs2010.sln with the following changes:
o targets .NET 4.5
o includes exactly the same sources as log4net.vs2010.sln
o defines an additional conditional compilation symbol FRAMEWORK_4_5_OR_ABOVE which is used to conditionally execute the new unit tests in the patch (Task.WhenAll used in the tests does not exist prior to .NET 4.5)
· Add a runtime check when accessing LogicalThreadContext.{Stacks,Properties} which refuses to run is the underlying .NET runtime is not at least 4.5
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
AW: [GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by Dominik Psenner <dp...@gmail.com>.
I see this targeted to .NET 4.5 only. Would this patch work also for .NET
4.0?
-----Ursprüngliche Nachricht-----
Von: ahouben [mailto:git@git.apache.org]
Gesendet: Donnerstag, 05. März 2015 02:53
An: log4net-dev@logging.apache.org
Betreff: [GitHub] log4net pull request: Fixes for the LogicalThreadContext
GitHub user ahouben opened a pull request:
https://github.com/apache/log4net/pull/12
Fixes for the LogicalThreadContext
The current implementation of `LogicalThreadContext` does not flow
correctly through `async/await` calls as described in
http://stackoverflow.com/questions/23316743/log4net-logicalthreadcontext-not
-working-as-expected.
This fix honors the shallow-copy-on-write behavior and immutable
requirements when using `CallContext.Logical{Get,Set}Data`. Please see the
accompanied `LogicalThreadContextTest` which is a copy of
`ThreadContextTest` but adds `TestLogicalThreadPropertiesPatternAsyncAwait`
and `TestLogicalThreadStackPatternAsyncAwait` to demonstrate the fix.
Thanks to @stephen-cleary for all the good information on the inner
workings of the async/await mechanism all over the internet.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahouben/log4net feature/async
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/log4net/pull/12.patch
To close this pull request, make a commit to your master/trunk branch with
(at least) the following in the commit message:
This closes #12
----
commit 72673fc46aa8abe4fd21018104352b143b7cce8d
Author: Alexander Houben <ah...@greenliff.com>
Date: 2015-03-05T01:17:28Z
Fix for the LogicalThreadContext{Stack,Properties}
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] log4net pull request: Fixes for the LogicalThreadContext
Posted by maxcherednik <gi...@git.apache.org>.
Github user maxcherednik commented on the pull request:
https://github.com/apache/log4net/pull/12#issuecomment-128709667
To my previous comment - I guess this behavior is by design.
This is the same way as in this [article!] (http://blog.stephencleary.com/2013/04/implicit-async-context-asynclocal.html)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---