asynkron / Wire

Binary serializer for POCO objects
Apache License 2.0
376 stars 63 forks source link

ObjectSerializer SpinWaits forever when BuildSerializer fails #115

Closed shivanan closed 7 years ago

shivanan commented 8 years ago

I've come across a situation where the BuildSerializer method on some unknown or odd type causes a failure in the expression compiler. This causes the serializer to remain in the _serializers dictionary but without being initialized. This by itself is not an issue.

However the problem comes if you later try to serialize the same type - instead of getting an error, the serializer essentially spinwaits forever, waiting indefinitely for _isInitialized to be set to true, which will never happen.

Several solutions:

  1. Add the object to _serializers only after its successfully initialized. Downside is that concurrent calls will end up creating wasted extra serializers.
  2. Add a catch block and remove the serializer from the dictionary if it couldn't be initialized. Also set some flag in the serializer so if any concurrent code is accessing it before it could get removed, the spinwait can check for this error flag and raise an exception - Requires changes in every place where the dictionary gets a serializer added to it - need to check for exceptions and remove the item.
  3. Just add a timeout to the spinwait - easiest solution. If it spins for more than a second, raise an error.

I was able to reproduce this scenario by accidentally trying to serialize a BsonDocument (from MongoDB). I think its ok to throw an error on unknown types, but it shouldn't spinwait indefinitely.

I'll be happy to submit a pull request for any of these three methods - would like some feedback on which is the preferred approach?

rogeralsing commented 8 years ago

Thanks for reporting. The best way to deal with this would probably be to have some sort of InvalidTypeSerializer that is just a no-op serializer, and if the object serializer cannot be created, the no-op serializer could be added to the _serializers lookup instead. That would be more explicit IMO, the type still has a serializer, everything works as normal, it just doesn't do anything.

Where do the actual exception occur? inside the generated read or write methods? If it fails during the generation, then everything would fail so it has to be one of the two above, right?

shivanan commented 8 years ago

Where do the actual exception occur? inside the generated read or write methods?

Yep, it occurs at DefaultCodeGenerator.GetFieldsWriter.

That would be more explicit IMO, the type still has a serializer, everything works as normal, it just doesn't do anything.

Maybe I didn't understand clearly but - isn't it better to fail noisily rather than silently no-op the serialization?

How would the caller know that something failed?

rogeralsing commented 7 years ago

Fixed in #118