boku-ilen / geodot-plugin

Godot plugin for loading geospatial data
GNU General Public License v3.0
109 stars 19 forks source link

Thread unsafety edge case when loading/using GeoImages on Windows #23

Open kb173 opened 4 years ago

kb173 commented 4 years ago

We're still not quite sure what exactly is causing it, but there can be crashes on Windows in very specific circumstances. It is apparently related to loading new (non-cached) GeoImages from multiple threads. Here are some of our observations while testing in LandscapeLab:

In https://github.com/boku-ilen/geodot-plugin/commit/2419613d212d0e0c7cd13c02e91458edacff804b, rigorous mutexes were added anytime a new resource is created or loaded. Interestingly, this did not fix the crashes, although they did seem to get less frequent.

With these mutexes and the cache, the creation of the GeoImage (loading GeoRaster, reading as array, etc) is actually non-threaded. That's why this behavior is really strange... I can't think of any situation where our two threads actually do something in parallel without a mutex, aside from GeoImage.get_image() and GeoImage.get_image_texture(). However, even those functions use a mutex internally.

This crash has never been observed on Linux, while it's pretty frequent on Windows (always happened within ~5 times of loading terrain up to a high LOD). So I'd say the issue is either related to GDAL on Windows, or to our compilation on Windows (which uses Visual C++, not g++).

There is no error message at all, the window simply closes.

We can currently work around this issue by using two copies of the same heightmap, one in the TerrainModule and one in the TerrainColliderModule. While this works, it's not very clean, so it would be good to figure out exactly what's going on.

(related to #10)