How threadsafe is System.Reflection.Emit?

Question

I'm wetting my feet with dynamic code generation and System.Reflection.Emit. All seems pretty easy and straightforward, but there's one question which I cannot find answered on the web.

When building dynamic assemblies with AssemblyBuilder it's possible to create a type and then start using it immediately. If later you need to add another type to the assembly, you can do that too and it's all fine (as far as I can tell).

But what if there are two threads that are trying to build types for that assembly? Or what if one ready-made type is already being used, while another is in the making? Will there be any conflicts or race conditions?

Added: OK, I think that a bit more information is necessary.

What I'm doing with Reflection.Emit is dynamically implementing interfaces at runtime. Basically I have something like:

class MagicClass
{
    public static T GetImplementation<T>();
}

Where T needs to be an interface (and a few other unrelated requirements).

Each interface will have exactly one implementation and that implementation will have exactly one instance. They will all be thread-safe singletons.

So when a request for a new, hitherto unseen interface comes in, I implement it, make an instance and then cache it for all eternity (which is here defined as "until my program is stopped").

This will however be done in a ASP.NET web application, so we have many threads requesting interfaces all the time. Performance is important and I'm trying to figure out how much multithreading I can afford.

It's pretty clear that a single interface will ever only be implemented by a single thread. But can I implement two different interfaces in two different threads at the same time? I guess the answer is - "better not".

OK, so I can add a lock that only one thread is doing an implementation at the same time. That will take care of builders interfering with each other. But what about those interfaces that were implemented previously? Other threads are using them at the same time while we're making a new one. I guess they're fine, unless they're trying to use some kind of reflection on their own assembly?

I could, of course, make one-assembly-per-interface-implementation policy, but that would spam the "Loaded Modules" debugger window real fast. Also, I don't know what performance effects there will be of having a 100 loaded assemblies (but I doubt that they will be good).

Added 2: Right, there must be something wrong with my language, since people don't seem to get it. Let me try with a code example:

var object1 = MagicClass.GetImplementation<I1>();
DoSomethingInAnotherThread(object1);
var object2 = MagicClass.GetImplementation<I2>();

Can there be a threading-related error between DoSomethingInAnotherThread(object1) and MagicClass.GetImplementation<I2>()?

We can assume that:

• DoSomethingInAnotherThread(object1) does NOT call MagicClass.GetImplementation<T>()
• DoSomethingInAnotherThread(object1) does NOT use reflection on object1 and neither does object1 itself.

Basically, the question is - can an innocent call to object1.SomeMethod() blow up because the assembly is being reorganized on another thread.

Atrioventricular

Is this worth referencing here? #18387530 – Syndicalism

Why not just write a simple test case and see? Also why does this need to be multithreaded? For a transient assembly, just create one builder per thread. – Briquet

@Briquet you can't really write a simple test case that proves whether something is thread safe; most thread-safety issues are uncommon race conditions, so a test can pass while the approach is fundamentally flawed – Unpractical

@MarcGravell: The intent of the test case is the actually try proof the opposite ;p With some carefully placed calls, it should not be that hard. Then again, what do I know about multi-threading? ;p – Briquet

@Briquet my point is: to do those carefully placed calls, you usually need to have a deep understanding of where you expect it to fail; otherwise you're just firing randomly. – Unpractical

@MarcGravell: That is true :) – Briquet

@Vilx-: Given a constructed type will never reference another constructed type (specifically from another thread, same thread is OK), it should be fine to just have a single (transient) AssemblyBuilder. This is how IronScheme is setup. From limited multi-threading tests, I have seen no issues popping up with this. I will try run some load tests for my web evaluator to confirm. – Briquet

@Vilx-: Test done, my setup does seem to fail but very infrequently. I think I know where the problem is though as the error is: Duplicate type name within an assembly. (thanks for letting me find a bug \o/) Will re-run after fix. Scheme code: gist.github.com/leppie/d0b3dc2ae20bd549463a – Briquet

After fixing the unique type name issue, I cannot repro any more issues. Here is my test case: gist.github.com/leppie/70de30f79ea66668e606 I added a little explaination what is happening in terms of creating types. I have run it with over 10000 'spawned threads' with no issue \o/ – Briquet

As a side note: it seems performance drops quite badly a more types are loaded in an app domain (talking 10k plus types). That may probably cause a headache if you hitting those numbers. – Briquet

Answer 1

The only declared comment is "Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.", so we can't assume much.

The following is entirely implementation details via inspection of the IL, and cannot be used for anything except anecdote.

Checking DefineDynamicModule, it uses a lock, as does DefineType - in fact, it uses the same SyncLock as the parent assembly builder.

Some things inside TypeBuilder do the same (DefineEvent, DefineMethod, DefineField, etc); but some things, like DefineCustomAttribute end up in extern code, so we can't tell; however, I can't see a lock on the way through, and it doesn't pass the lock the the extern method.

Overall, it looks as though some thought has been given to thread-safety, but most likely they didn't want to formally guarantee it. It looks like most common stuff should be ok, though, especially if you never have multiple threads working on the same type.

Emphasis: this is all completely implementation specific, and we have no set of "this use-case is guaranteed to work, but this has not been considered".

I tend to keep it simple, personally; one builder (per assembly) is usually fine.

Unpractical

I'll make sure that only one thread at a time is building a type (that's also necessary due to some other unrelated requirements). I was more wondering if using the types which are already finished might conflict with generating a new one at the same time. – Atrioventricular

@Vilx TypeBuilder? or constructed Type via MakeType() (or whatever the method is)? Frankly, though, I'd advise "don't" – Unpractical

@Vilx-: If one type is not referencing another type being built, there should be no issue. The problem from what I can see in the other question is calling CreateType when a referenced type is still incomplete. – Briquet

OK, added clarification to the question. – Atrioventricular

If the code is internally locked anyway we can wrap an outer lock around it to be safe at no loss of scalability. – Vinasse

Answer 2

It is not specified.

Whenever you use code that is not explicitly documented to be thread-safe then you'll have to make a choice. Weigh the cost of not locking, getting it wrong and have your code crash in an impossible to diagnose way at utterly random intervals. Against the cost of taking a lock and be sure to never be wrong, just a little slow.

Where "little slow" is something you have to know to make the proper choice. It is an easy one in this case, building a method takes a few handful of microseconds, at most. That lock you put around it is not going to be contested. And if it is anyway then the delay that's incurred is unobservable to anybody. A thread context switch or a page fault, normal things that happen that cause code to run slower, take more time than that.

Take the lock. You'll never have to be sorry.

Karr

Please read the "Added" part. There's no problem with adding a lock to the building part. In fact, that's what I'm doing now. The question is - can I use things that were built in the past, while building a new one at the same time? – Atrioventricular

Also, the Added 2 part. – Atrioventricular

It is unclear to me what else you are fretting about. Thread-safety of the actual code you generate follows the normal rules of thread-safety for any code you write. If you implement "one interface per thread" then there's little to worry about. Clearly you can't violate thread-safety if the object's members are only ever accessed by one thread. – Karr

Well... I guess so. :) Thanks. :) – Atrioventricular

Answer 3

basically the question is can an innocent call to object1.SomeMethod() blow up because the assembly is being reorganized on another thread.

From experiences with the Castle.Core package, which uses System.Reflection.Emit to generate types, I am quite sure that yes, it always can and sometimes will blow up. The symptom I typically see is that you get either TypeLoadException or BadImageFormageException at the time you're trying to use one of your generated types.

Chrisy