Rychard / GnomeServer

Integrated Web Server for Gnomoria
MIT License
4 stars 3 forks source link

Provide thread-safe access of Gnomoria objects #2

Open Rychard opened 9 years ago

Rychard commented 9 years ago

Throughout development, I've encountered only a handful of crashes that were the direct result of accessing objects across thread boundaries.

Modifying existing objects seems to be safe (in practice, but this is not an acceptable excuse), but creating or removing items that exist as part of a collection can easily cause Gnomoria to crash if those collections are enumerated frequently, or if enumeration within the Gnomoria process takes an extended period of time to complete. The issue is unlikely to occur in reverse, as the web server itself is only executing code against Gnomoria when a request is received, and the Try/Catch blocks that surround the invocation of this code would capture any exceptions thrown in this scenario. The server would send the error as the body of the response, but the Gnomoria process is unlikely to be impacted.

A common scenario that can reliably result in a crash of the Gnomoria process involves the addition/creation of new Character objects. The collection that keeps track of these references is regularly enumerated across many of the game's Update methods, and modifying this from the web server thread (while Gnomoria is enumerating the collection) would throw an exception.

Note: For the uninformed, modifying a collection while it is being enumerated will cause the following exception to be thrown:

System.InvalidOperationException was unhandled
  Message="Collection was modified; enumeration operation may not execute."
  Source="mscorlib"
  StackTrace:
       at System.ThrowHelper.ThrowInvalidOperationException(...)
       at System.Collections.Generic.List`1.Enumerator.MoveNext()

During the execution of the GnomoriaInjection.Inject.Hook method, we have access to the game's primary thread, and can create a dispatcher at this point, but this does nothing to provide us with the thread-safety we need. Since this method is invoked any time the GnomanEmpire.Instance property is accessed, Gnomoria could be directly in the middle of enumerating one of these collections.

Instead, it might be wise to instead use a second hook that is invoked at the end of one of the game's many Update methods. The master Update loop should be safe to hook into, since I can't imagine a scenario that involves a collection being enumerated at this time.

Rychard commented 8 years ago

I attempted to add a second method call to GnomoriaInjection.Inject called Update that would be called one of the Update method in Gnomoria.exe but it seemingly prevented the game from loading. I'm unsure why this is, but I'm certain there's some explanation for this.

Finding an alternative (workable) solution to this would allow us to drop the requirement that the game be in a paused state before doing things like creating Gnomes, which could potentially crash the game otherwise.

jkluch commented 8 years ago

(ItsComcastic on the forums) I don't think pausing the game before making changes is that big of a negative. I personally only toyed with changing skill levels and Gnome names, but if pausing the game fixes potential crashes any game updates can just get pause state, pause if running, run code then call unpause if game was running.

Rychard commented 8 years ago

On the forums there was another individual who found some way to inject code dynamically across the entire assembly. If I recall correctly, it was a mod framework of sorts.

I believe that code is released on Github as well, it might be worth looking into, so long as the license on that repository permits it.