It depends on what kind of variables parentObject & parentObjectCopy are. If parentObjectCopy is not shared between threads ( private static ) then the lock is kind of useless since other threads will not block on it and can set partenObject to null while other threads try to call the indexer.

When a context switch happens right after the first if statement, but before the lock, another thread could execute "parentObject = null;". When the 'first' thread then resumes control, it will try to lock on a parentObject which is now null.

That's not it. There's a dataflow dependence so the compiler would never make that optimization. Also, on the CLR, all stores have release semantics, meaning there's an implicit barrier that does not allow stores to be reordered with other stores.

If some other piece of code sets parentObject to a different (non-null) value, then the parentObject you lock on might not be the same parentObject you use a few lines down. This might cause a NullReferenceException (and other strange behavior).

This is far-fetched, since the snippet doesn't show anyone setting parentObject, but still... Locking an object that can be replaced (by null or whatever) doesn't seem like a good idea to me.

lock() is not safe against reentrant code on the same thread. So maybe the implementation of properties.Clone() recurses to the block of code in question (and does not infinitely recurse, since we didn't get a stack overflow; suppose it has some sort of recursion limit). It will re-enter the lock, Clone() once more, null out the parentObject (I'm assuming it's not a local variable on the stack, since you're locking on it), then unwind to find parentObject null. Boom.