I know this topic has been raised before, and we all know you can’t catch a StackOverflowException (for obvious reasons), but the fact this error is thrown in the ServiceStack JSON serializers is a real issue.
Obviously we do everything we can to avoid ever serialising an object with a self-referencing property, but these are not always avoidable.
In our case we have a hosted Javascript scripting language, and users define the result, which we receive as an ‘Object’ and have to serialise to pass to other handlers. We have no way of knowing if they’ve done something that returns a cyclic dependency, but it’s not acceptable to have the entire service crash and reboot.
At the moment we have to replace ServiceStack with NewtonSoft calls because that library throws a catchable exception, but obviously we don’t want to do that.
Is there a reason why the depth can’t be tracked during the serialisation and always honour the MaxDepth? Is this a performance thing? “You should fix the self-reference” is not a realistic solution to this issue 
ServiceStack Serializers are not designed to be able to maintain state during serialization. We’re never going to support circular references which should never be in serialized objects - it’s an anti-pattern that’s a source of issues. We do have a TypeSerializer.HasCircularReferences(object) you can use to detect circular references as well as a ToSafeJson() which basically skips any properties which contain a circular reference, it’s not a solution because it will only partially serialize the object paths that it detects doesn’t have immediate circular references. The solution will always be to never serialize objects containing circular references.
I don’t think anyone expects any system to ‘support’ circular references, but causing a complete uncatchable application crash when one occurs is also not acceptable.
Does TypeSerializer.HasCircularReferences honour the JsConfig<>.RawSerializeFn functions? Obviously if a mapping exists that should be followed to know if a circular reference has already been avoided.
I’ll look at this, and if is as simple as replacing the call with a check then a call then that will do nicely, but overall it seems crazy to not detect and throw such a common situation. Is this just a performance concern? Why does MaxDepth not always work?
No HasCircularReferences does not call serialiser functions. To detect circular references properly you need to store a reference to every instance, which isn’t something we’ll do. Circular references causes StackOverflow Exceptions the same as calling a never ending recursive function which crashes all Apps. The solution is always to remove the StackOverflow Exception, not try “detect” whether each function call can cause one.
But we don’t need to detect circular references, we need the serialiser to detect an unacceptable depth of recursion, which is a different thing. As it stands you might as well replace any unexpected serialisation failure with an Application.Stop statement, because the supplied object must be serialisable. What if I try to serialise a disposed object, or if the external object has a property that raises an exception?
It sounds like you’re suggesting there is no way of avoiding a catastrophic system level exception when external objects are serialised by the SS serialisers, but NewtonSoft manage to do this. I’m asking why the ServiceStack model can’t handle counting the recursion depth to provide a means of raising a catchable exception that doesn’t wait for the PC’s stack to overflow. That’s why I keep asking if this is a performance concern, in which case I’ll take reliability over optimisation.
For now we have no choice but to use NewtonSoft, because we can’t have our server restarting the application service repeatedly (which is the issue we’re experiencing). This way a simple try-catch around the function accepting an external unverifiable object works exactly as expected and required.
I have no idea why you’re comparing this to a normal exception, it’s not. It’s not a temporal or unreliable Exception, it will always happen as a result of a design flaw in your code that tries to serialize circular references. The solution is the same as any StackOverflow Exception, identify and remove it.
The only way to avoid it is to try detect it before it happens. The Serializers was never designed to support circular references, simple as that. All serializers will throw a StackOverflow Exception because that’s the default behavior when serializing object graphs that never end. Trying to detect it would be disruptive (essentially requires a rewrite to adopt a different approach that flows state) and incur a perf hit neither of which I’m going to do to support a use-case that shouldn’t exist.
I’m not going to spend any more time on this thread. The Serializers have never and will never support circular references. I don’t care if other serializers support it, it’s a design flaw which should never be in DTOs or serialized objects. If you choose not to fix your code than you would need to use a different serializer.
Sorry, but I don’t think you’re reading what I’m typing.
NewtonSoft does not support circular references either.
NewtonSoft does not crash an application if a circular reference is detected.
It would be naive and presumptive to tell people “Your users must be educated to never generate output that has a circular reference otherwise it will crash the application”. That is not a “use case that should not exist”, it’s more of a “real-world situation that I would prefer not to have to deal with”
That’s fine, NewtonSoft handles this fine, I just thought you might like to know where the SS implementation has an issue that makes it unusable in situations involving un- testable input, and a thread like this is important for clarification of the situation for anyone else searching for solutions to this problem.
I read what you’re saying fine, you want the libraries you use to expend resources trying to catch and identify StackOverflow Exceptions from occurring before they happen so they yield a catchable Exception for what would otherwise always yield a StackOverflow Exception.
No I’m not saying there should never be a bug, it should be handled the same way as any other StackOverflow Exception is handled and how every other user handles it when they’ve encountered a StackOverflow Exception when using ServiceStack Serializers in the last 8 years the library has existed - identify and resolve the issue.
This is not a real-world situation that needs to be supported, it’s a bug that should be resolved. To that end you can use TypeSerializer.HasCircularReferences() to help detect circular references.
I’m always open to suggestions.
Perhaps you can suggest a solution to the following (pseudocode-esque)
ClearScriptV8 context = new V8ScriptEngine();
ctx.AddType<ComplexUserObject>(); // (This is my owned type has a property of type 'IManager' that is known to be recursive, so I have to add a customer serialiser)
JsConfig<IManager>.RawSerializeFn = c=> ManagerSerialiser(c);
do {
var cmd = Console.ReadLine();
var resultobj = ctx.Evaluate(cmd);
// Returns an arbitrary object structure generated by ClearScript, some basic types, some array, and possibly some of type IManager (which are correctly serialized by ManagerSerialiser )
try {
Console.WriteLine(JsonConvert.SerializeObject(resultobj))
}
catch(Exception e) {
Console.Error.WriteLine("Problem: " + e.Message);
}
} while(true);
Let’s say the user enters some perfectly valid code such as:
var a = new ComplexUserObject();
a.Manager=a;
return a;
How would you prevent this crashing the whole application? I’m stumped as to how it can be handled. I can’t use HasCircularReferences because that will always report a circular reference even though they are valid and handled by ManagerSerialiser. I can’t use ToSafeJSON because this calls HasCircularReferences.
This is not a bug, it’s a functional requirement. What is it that I’m missing here? How can I guarantee my program won’t crash back to the OS?
Just as a possible option, if custom serializers are defined that handle the serialisation of objects externally, can they not be detected by HasCircularReferences and ignored, as you know they can’t be circular because they are returned as strings? The external call doesn’t need resolving during a test because it has its own responsibility to avoid recursion.
Just skimming this quickly, @jsobell would it be possible to, rather than use the SAME object when doing your point-to-self-to-some-depth, in which case you get a circular reference, could you not simply clone the data that you need?
e.g something like:
var a = new ComplexUserObject();
a.Manager = a.cloneToEntirelyNewObjectThatLooksLikeAButIsnt(); // so that its not the same object that would get stuck in endless recursion
return a;Make a finite DTO for this and populate the DTO with the original object. Somewhat like DaleHolborow mentioned. Serializing internal objects is prone to circular references.
Serializing internal objects and expecting them to work is bad practice: always use DTOs for external serialization.
For example:
ComplexUserDTO has 1 Manager property, with an ID. This will be sufficient
What Rob said, copy it into a serializable DTO and serialise that. Objects with circular references are not supposed to be serialized.
As I said, I’m always open to suggestions. So I just walk down every single shared object the user has access to on every single request, then create proxy instances. I’ll just create proxies for every single DTO and referenced object, resolve every calculated property in advance, and pray that the users don’t call any functions that return an original object. I suppose I could replace every function and property on every proxy object with instances that generate their own non-recursive objects, collections and hierarchies.
Or perhaps that’s really just not a realistic suggestion.
When I said “pseudocode-esque”, I meant this is a simplified example, not that I have one property on one object in an application that needs different handling 
As Rob says
Serializing internal objects and expecting them to work is bad practice: always use DTOs for external serialization
I’m 100% with you on this one! I don’t want or expect them to work, I just don’t want the server to crash when one is unavoidably experienced.
Just to give a clearer understanding of why this is a requirement in real-world situations (and is likely to become more common), can you imagine cloning the User object?
or what about elements of an external library the user has included themselves?
Ah, if only everything in the real-world was neatly wrapped up in a bubble of tidiness 
It’s important to note that there is a very valid reason why NewtonSoft trap this situation. To suggest it’s to allow people to do bad programming would be naive and presumptuous, and it’s implemented precisely to resolve real-world issues like the one I’m showing.
You just copy what you need. If you can’t even copy what you need from the object, why would you think trying to serialize an object that was never meant to be serialized is somehow the appropriate solution? There’s no chance you’re going to be able to deserialize the object so I don’t really know what you’re trying to accomplish.
But if you’re just trying to dump the contents of a V8 JS object, you can try calling JavaScript’s JSON.stringify() and use that JSON instead.
Your examples are just reasons of when serialization is an invalid approach, serializing an internal JS Object’s API methods is not a real-world requirement, it’s an implementation detail of an invalid approach to whatever you’re trying to do which is definitely not representative of a valid use-case serializers should support. It’s hard to think of a worse object graph that you should attempt to serialize.
why would you think trying to serialize an object that was never meant to be serialized is somehow the appropriate solution?
And that is exactly why I said you’re not reading the content of my posts properly. You’re fixating on the assumption that I want some serialised data back from a serialisation call.
So for the fifth time, I don’t want any data back, I want the application to catch the error without crashing back to Windows!
But if you’re just trying to dump the contents of a V8 JS object, you can try calling JavaScript’s JSON.stringify() and use that JSON instead.
Yes, when I first hit this issue with SS was my idea too, but it doesn’t work, because when the user uses an object as a nested recursive call out of ClearScript it will not be wrapped in JSON.stringify(). ClearScript’s stringify function can’t serialise .NET objects, so it has to use an external library, and change deserialisers based on the object type (which is why we use JsConfig<JSValue>.RawSerializeFn in our code).
Every single library function in the framework takes responsibility for preventing it’s own internal stack-overflow errors. My code is responsible for stack overflows in areas where I have control over. I have no control over the stack overflows that may occur within the ServiceStack serialisation functions.
Every other type of exception in .NET can be captured. Sure, the data is not serialised (did I already mention that I don’t expect it to be?), but the result of those calls is an Exception being raised. StackOverflowException is the worst nightmare to occur in any program, and is the equivalent of the Blue Screen of Death. If I manage to write my code in such a way that this occurs, it’s completely my fault, and it’s my job to track and check for this happening (and I do in several places where recursion can happen due to users’ dodgy configuration). If I call an external function that takes an arbitrary object, it’s that functions responsibility to either perform the operation or report that it can’t.
I’m assuming you really don’t want to have to change the library to resolve this because you consider this an ‘edge case’. That’s fine because the workaround is to use NewtonSoft’s library. It’s just annoying to have to use someone else’s library to address this issue, and I think very dangerous to know that a tiny oversight in a rarely used object somewhere in a library might completely crash an applications.
I’ll continue using the SS serialisation in simplistic situations where I know the exact objects being handled, but it simply can’t be used for something where the user can define their own objects or control the objects to be serialised.
Then stop trying to serialize it! (as I’ve kept repeating in every comment on this thread) Seriously, I can’t be any clearer than this. Don’t serialize objects with circular references, ever. The behavior you’re getting is exactly the behavior that’s supposed to happen which should be a giant red flag that you’re doing something very wrong. It’s not an edge case, it’s just wrong, exactly the kind of thing that you shouldn’t be serializing and a perfect example of when to use DTOs (Data Transfer Objects) - a clean, separate model for the purposes of serialization.
That’s the wrong take away, you’re searching for libraries to compensate for bad decisions, trying to hack your way around issues that shouldn’t exist for doing things you’re not supposed to.
Do whatever works for you, as I’m seriously done with this thread.
OK, well I stepped through all of the ServiceStack code to find how and why it occurs, mainly because I wanted to understand why your MaxDepth was not being triggered (as it appears to be doing almost exactly what I’m requesting).
It turns out that MaxDepth is only tested when properties directly on the object are serialised, and not when a collection is iterated over. In this case we have a property result in an external library containing an ‘IEnumerable’ object masquerading as a ‘String’. When your code reads it, it reads it as a KeyValuePair, and proceeds to write out the value for each item in the collection. That’s fine.
But, when it goes to resolve the Value of the KVP, the external library says it’s an IEnumerable of one character, so your code writes out the Key then proceeds to resolve the Value in the KVP, which is an IEnumerable… etc.
So the issue is that the depth check needs to also be checked in the functions such as WriteGenericEnumerable, as this is where the code can spin out.
Here’s some sample code that breaks the MaxDepth check:
public class Thing : Collection<KeyValuePair<string, Thing>> { }
static void Main(string[] args)
{
var data = new Thing();
var kvpitem = new KeyValuePair<string, Thing>( "X", data);
data.Add(kvpitem);
JsConfig.MaxDepth = 5;
var t = ServiceStack.Text.JsonSerializer.SerializeToString(data);
}
One thing I would say though is that I don’t believe MaxDepth is correct in returning the serialised object terminated at the specified depth, because it’s not actually the serialised object. MaxDepth might be better called PruneDepth, as I would expect MaxDepth being exceeded to raise an Exception, otherwise how do we know if an object is fully serialised or just a shallow copy?
I’ve added MaxDepth checks when traversing lists in this commit. This change is available from v5.1.1
MaxDepth is appropriately named, it indicates the Max Depth the serializer should go down before it stops trying to serialize the object, the same as if you’re searching a tree, it only recursively descends the max depth for finding an object.
You don’t get to know if it reached the Max Depth, only that it shouldn’t exceed it. But it does generate an Error message which you can view by injecting a tracer, e.g:
Tracer.Instance = new ConsoleTracer();
Alternatively you can set JsConfig.ThrowOnError=true to have Error Messages throw Exceptions. Or if you prefer ServiceStack was more strict and throw Exceptions on Errors instead of logging you can enable Strict Mode in ServiceStack with:
SetConfig(new HostConfig {
StrictMode = true
});
Which is an alias for:
Env.StrictMode = true;
Which acts like a global switch to make ServiceStack stricter and fail fast for different error conditions.
Awesome stuff, speedy as ever!
Re MaxDepth, it’s nice to see there is a workaround. I would have assumed the appropriate behaviour would be like this: https://www.newtonsoft.com/json/help/html/MaxDepth.htm as I’m not sure what you would ever use a truncated depth tree for, and have never seen it done before.
Hang on! Perhaps it’s so you can successfully serialise the circular-referencing objects that I think should generate a catchable exception!!! 
Thanks for the help!