Scope config #253

Merged
merged 4 commits into from Dec 12, 2012

Conversation

Projects
None yet
4 participants
Contributor

paultyng commented Dec 12, 2012

No description provided.

@mythz mythz added a commit that referenced this pull request Dec 12, 2012

@mythz mythz Merge pull request #253 from paultyng/scope-config
Scope config
fe3833a

@mythz mythz merged commit fe3833a into ServiceStack:master Dec 12, 2012

Owner

mythz commented Dec 12, 2012

Sweet this is good stuff, thx!

Consider,

public object Get(Dto dto)
{
   using (var scope = new JsConfigScope())
   {
      scope.EmitCamelCaseNames = false;

      return GetSomeObject();
   }
}

If the configuration scope is disposed after the using block exits, but the response object isn't serialized until after the current method returns how is this supposed to work? Is there a unit test that covers the intended behavior?

Per the link below, [ThreadStatic] is inappropriate unless you control the thread pool, but in ASP.NET your request is processed on an ASP.NET worker thread. So this JsConfigScope class will randomly affect other requests happening concurrently which are being serviced on the same thread. It doesn't look like the code takes this into account.

http://www.hanselman.com/blog/ATaleOfTwoTechniquesTheThreadStaticAttributeAndSystemWebHttpContextCurrentItems.aspx

Owner

mythz commented Apr 18, 2014

Try to understand what's happening.

You're setting up a config scope for the thread, setting some configuration on the threadscope returning an object then disposing the thread scope.

What's not happening is any serialization as you're just returning an object, so this code is meaningless as the scope is cleaned up before any serialization occurs, resulting in the configuration having no effect. What's also not happening is:

So this JsConfigScope class will randomly affect other requests happening concurrently which are being serviced on the same thread

How can any other thread but the running code use that thread until its finished? Do you think that between normal sequential code your running thread gets hijacked and runs some other code? That's not how it works, the worker request gets reused if the thread gets yielded or after the request ends, neither happens when running normal sequential blocking code like the above.

Do you think that between normal sequential code your running thread gets hijacked and runs some other code?

Yes. In ASP.NET, that is what happens.

Under load an ASP.NET worker thread processing a request may be interrupted and re-tasked to process a different request before returning to the first request. The unit of isolation for an ASP.NET request is not [ThreadStatic] it's the HttpContext object.

It's not immediately obvious that ASP.NET works like this, and it's bit me in production before. Read Hanselman's blog post.

This code needs to be rewritten to use HttpContext.Current.Items. Unfortunately, this has implications for cases where shared business logic is targeted by multiple hosts because HttpContext.Current will be null in cases where ASP.NET is not present.

There are some .NET BCL features that are incompatible with ASP.NET because they rely on ThreadStatic variables. For example, setting the culture of the current thread may appear to be a cheap win for globally affecting the behavior of the DateTime and currency string formatters, but you'll end up sending Spanish to English clients under load if those pathways are surfaced through ASP.NET.

What's not happening is any serialization as you're just returning an object, so this code is meaningless as the scope is cleaned up before any serialization occurs, resulting in the configuration having no effect.

I agree that the configuration has no effect on the behavior of the serializer. Given that, what is the intended way to use JsConfigScope? I don't understand how this capability is supposed to be used regardless of its high load hazard.

Owner

mythz commented Apr 18, 2014

Yes. In ASP.NET, that is what happens.

Under load an ASP.NET worker thread processing a request may be interrupted and re-tasked to process a different request before returning to the first request.

Eh Sorry what? You're saying the thread gets transparently interrupted like throws a ThreadInterruptedException? Hanselman's post just says that threadstate is shared with the worker thread, not that the thread gets re-used in midflight. If that was the case where you can magically re-use a thread in midflight than we wouldn't need async code because blocking IO wouldn't tie up the thread and could be re-used while it was blocking, which negates the whole benefit of async non-blocking IO.

No the code doesn't need to be rewritten 1) the code is still meaninglesss and would have no impact even if it was using HttpContext.Items 2) Scoped ThreadStatic code works as intended.

No, I'm saying that the ASP.NET may move a request between threads during its processing lifecycle. The term is called thread-agility. Look, there are a dozen articles on this in Google.

Also, we agree that the code is meaningless. How would I use this feature correctly?

Owner

mythz commented Apr 18, 2014

I know all about CallContext / HttpContext.Items which is what ServiceStack's RequestScope uses. But what you're saying is a thread can be interrupted and re-used in midflight and that somehow in the middle of between Creating and disposing the scope the implementation is somehow run on a different thread which isn't the case. It only can run on a different thread in during a switch, which if you read the page spells out when these events occur:

  • The thread switch occurs after the IHttpHandler has been created
  • After the page's field initializers and constructor run
  • After any BeginRequest, AuthenticateRequest, AquireSessionState type events that your Global.ASA / IHttpModules are using.
  • Only the HttpContext migrates to the new thread

What's also missing is awaiting async code which can continue execution on a different thread.

It does not mean thread switches occur in the middle of your running code and that potentially each subsequent statement could be run on a different thread.

Maybe we should look at an example of how JsConfigScope is intended to be used?

We agree that the above code is meaningless; to fix it one option is to install the scope as an aspect or as a filter at an earlier point in the request processing pipeline (e.g. BeginRequest). That puts an ASP.NET managed call boundary between the scope creator and the request handler.

If at that boundary, the ASP.NET runtime decides to reschedule the current request onto a different thread due to high load, then the HttpContent is moved to the new thread, but the ThreadStatic variables which ASP.NET does not know about will not be moved.

EDIT: Clarity

@politician You can try this filter attribute on your service method:

public class Iso8601DateSerializationAttribute : Attribute, IHasRequestFilter, IHasResponseFilter
{
    [ThreadStatic]
    private static JsConfigScope _configScope;

    public void RequestFilter(IHttpRequest req, IHttpResponse res, object requestDto)
    {
        _configScope = JsConfig.BeginScope();
        JsConfig.DateHandler = JsonDateHandler.ISO8601;
    }

    public void ResponseFilter(IHttpRequest req, IHttpResponse res, object response)
    {
        _configScope.Dispose();
    }

    IHasResponseFilter IHasResponseFilter.Copy()
    {
        return new Iso8601DateSerializationAttribute();
    }

    int IHasResponseFilter.Priority
    {
        get { return 0; }
    }

    IHasRequestFilter IHasRequestFilter.Copy()
    {
        return new Iso8601DateSerializationAttribute();
    }

    int IHasRequestFilter.Priority
    {
        get { return 0; }
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment