Thread-safety of Protobuf-net Serializer.Serialize<>

[GoogleCodeExporter]

protobuf-net rev: 349
.NET Runtime Version v4.0.30319
Dual core hardware

It seems Serializer.Serialize<> suffers from a race-condition. Being a static 
method I expected it to be thread-safe, but it seems it is not. I observe 
various problems, ranging from exceptions to malformed buffers, unless I 
explicitly lock a global lock while using the Serializer singleton.

Typically a InvalidOperationException is thrown with the following stasck trace:
   ved ProtoBuf.Meta.MetaType.ThrowIfFrozen() i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\MetaType.cs:linje 168
   ved ProtoBuf.Meta.MetaType.Add(ValueMember member) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\MetaType.cs:linje 696
   ved ProtoBuf.Meta.MetaType.ApplyDefaultBehaviour() i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\MetaType.cs:linje 288
   ved ProtoBuf.Meta.RuntimeTypeModel.FindOrAddAuto(Type type, Boolean demand, Boolean addWithContractOnly, Boolean addEvenIfAutoDisabled) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\RuntimeTypeModel.cs:linje 121
   ved ProtoBuf.Meta.RuntimeTypeModel.GetKey(Type type, Boolean demand, Boolean getBaseKey) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\RuntimeTypeModel.cs:linje 252
   ved ProtoBuf.Meta.RuntimeTypeModel.GetKeyImpl(Type type) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\RuntimeTypeModel.cs:linje 248
   ved ProtoBuf.Meta.TypeModel.GetKey(Type& type) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\TypeModel.cs:linje 561
   ved ProtoBuf.Meta.TypeModel.SerializeCore(ProtoWriter writer, Object value) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\TypeModel.cs:linje 135
   ved ProtoBuf.Meta.TypeModel.Serialize(Stream dest, Object value) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Meta\TypeModel.cs:linje 155
   ved ProtoBuf.Serializer.Serialize[T](Stream destination, T instance) i D:\Devel\stuff\hg\Gateway\Protobuf-net\protobuf-net\Serializer.cs:linje 69
   ... my code

It is not explicitly documented, I gather from earlier issues, that you 
intended the Serializer singleton to be thread-safe (but that might have been a 
V1 feature).

Serializer.Deserialize<> does not seem to suffer from similar problems.

Original issue reported on code.google.com by kasper.o...@gmail.com on 3 Sep 2010 at 12:57

[GoogleCodeExporter]

Hmmm - an interesting v2 bug. Indeed, this should work. I'll investigate. As a 
workaround, I'd be interested to know if calling the `PrepareSerializer<T>` 
method (early on in the app) fixes this for now. (emph: workaround; I plan to 
resolve this properly)

Original comment by marc.gravell on 6 Sep 2010 at 6:19

[GoogleCodeExporter]

Calling PrepareSerializer<T> for all involved T seems to fix the race.

BTW: My earlier comment about Serializer.Deserialize<> was wrong, it can happen 
as part of deserialization as well.

Original comment by kasper.o...@gmail.com on 6 Sep 2010 at 10:05

[GoogleCodeExporter]

I can confirm the same problem.
I'm using nongeneric function and while deserializing using multiple threads i 
get random exceptions.
I've fixed it using suggestion posted in comment 2;
after porting the PrepareSerializer function to NonGeneric.cs and using it to 
inizialize all T the problem vanished.
Using protobuf-net rev. 359 under .Net 3.5

Original comment by tipolo...@gmail.com on 9 Feb 2011 at 5:03

[GoogleCodeExporter]

Any code I can use to repro would be appreciated...

Original comment by marc.gravell on 9 Feb 2011 at 9:27

[GoogleCodeExporter]

Here is a sample program that reproduces the problem.
On the first run it creates 16 files with some serialized data.
On the following run it read the files in memory stream (sequentially), then it 
launch 16 threads that will deserialize the streams in parallel.

Please run it twice to reproduce the problem.

Let me know if you need the whole project, altough the code is very simple and 
it only depends on protobuf-net.

Original comment by tipolo...@gmail.com on 10 Feb 2011 at 10:41

Attachments:

• Program.cs

[GoogleCodeExporter]

Obviously the program i've posted reproduces the problem on the Deserialization 
side, altough i think the problem described by the bug opener is the same as 
mine.
P.S. Can you add PrepareSerializer in the NonGeneric.cs ?
I've added it as a quick fix for my project.

Original comment by tipolo...@gmail.com on 10 Feb 2011 at 10:45

[GoogleCodeExporter]

In v2, PrepareSerializer is actually synonymous with:
    RuntimeTypeModel.Default[typeof(T)].CompileInPlace();
so you should find that:
    RuntimeTypeModel.Default[someType].CompileInPlace();
does the job

Original comment by marc.gravell on 10 Feb 2011 at 1:32

[GoogleCodeExporter]

Yes, i've seen that; i've suggested the Prepareserializer implementation in 
non-generic version just to be similar to the PrepareSerializer<T> 
implementation present in generic version.
Obviously that's only a suggestion :)

Do you have tested my code? Let me know if you need something different..

Original comment by tipolo...@gmail.com on 10 Feb 2011 at 1:39

[GoogleCodeExporter]

I can add a PrepareSerializer in v2, sure. I'll get to the attachment when I 
have a second; haven't had chance to try it yet. I'm sure it will be 
illuminating.

Original comment by marc.gravell on 11 Feb 2011 at 11:20

[GoogleCodeExporter]

I've tracked down the problem to a race condition present in FindOrAddAuto, in 
RuntimeTypeModel.cs
If two threads are trying to deserialize an object of some unprepared type,
both thread will try to prepare it, leading one of them throwing an exception 
because the other has frozen the type.
I've applied a lock around all code inside FindOrAddAuto and the problem 
vanished.
This imply that also the type find is interlocked, maybe losing a bit of 
performance, but at least for me it solves the bug.
Hope to be helpful :)

Original comment by tipolo...@gmail.com on 15 Feb 2011 at 2:58

[GoogleCodeExporter]

Much obliged;

Original comment by marc.gravell on 15 Feb 2011 at 3:39

[GoogleCodeExporter]

any news on this?

Original comment by tipolo...@gmail.com on 11 Apr 2011 at 9:45

[GoogleCodeExporter]

all fixed

Original comment by marc.gravell on 13 May 2011 at 8:07

• Changed state: Fixed
• Added labels: OpSys-All

[GoogleCodeExporter]

Is this bug on the v2 branch and/or the v1 branch?  We are currently using 
version 1.0.0.282 on 12 core + hyperthreading servers.

Original comment by ray.rizzuto@gmail.com on 30 Aug 2011 at 8:37

[GoogleCodeExporter]

@ray.rizzuto neither, since it was fixed months ago. But it *used* to relate to 
v2. The servers we use for stackexchange *also* have a great deal of 
parallelism, and we have been using it there *extensively* for many months.

Original comment by marc.gravell on 31 Aug 2011 at 5:25