Workspace.Windows collection is not accessed/updated atomically

[josteink]

I often get the following exception while running workspacer:

We either need to provide locked access on object, or make updates atomic.

We should probably also replace IEnumerable<> with an explicit Array-type or List-type, to be better able to control enumeration and avoid situations like this.

Unless someone else beats me to it, I will probably try to debug this more in depth and find out where the collisions occur, and possibly tighten up the code.

[rickbutton]

Yep, workspacer does a lot of things non-atomically all over the place, and mutates global state on a bunch of threads.

This is one of the places where the initial design failed to fulfill the requirements of the full application, so things got a little hairy, although it doesn't cause problems usually.

I'm open to suggestions as to how to improve this, short of gutting the entire application in a refactor (not opposed, but would enjoy some direction first).

[josteink]

Ok. So based on the branch I've submitted as "Managed Windows", I've been able to reproduce this error, and I think fixing it is pretty straight forward.

I'll update the PR to include those fixes, and then merging that PR should also close this bug.

So I'm working with Windows, and then VS breaks workspacer into Debug mode:

Looking into parallel stacks there are only 2 concurrent stacks working with Windows:

What's going on in the first thread is pretty obvious: We're enumerating _windows and it's being modified underneath our feet.

Looking at the other stack, I just went down a few frames until I found this:

And currentWorkspace here is the same workspace as the one producing the exception! (Notice it's the same object, with object-id $1).

So basically what we need to do is add the following around the code inside AddWindow(), RemoveWindow() and the ManagedWindows-property:

lock (_windows)
{
   // code
}

And this problem should be history.

Any objections?