Make projection context ref counting thread safe (#7079)
@szekerest I'm wondering if it is not hiding a more serious concern: a PROJ context should not be used by multiple threads simultaneously. So if you need to make ref counting thread-safe, this is a sign that a PROJ context might be used simultaneously at some point. Can you share more details about the issue?
@rouault According to the stack traces attached to the corresponding ticket, the handling of refcount in msProjectionContextUnref is not thread safe so proj_destroy can be called multiple times. This causes an issue of a client running an ASP.NET (mapscript) app on the server under a heavy load condition and causes the host process to crash approx 20 times a day. They have just reported that by applying this fix, the issue has gone after 3 days of continuous operation.
the handling of refcount in msProjectionContextUnref is not thread safe so proj_destroy can be called multiple times.
yeah, I've no doubt that this fix improves things, but I'm wondering if it is not just a workaround for a more serious issue, and multi-threading bugs could still happen (perhaps less frequent) I suspect msProjectionInheritContextFrom() should refuse to reuse the initial context if the threads are different, or perhaps clone more things. I'll try to come up with an alternative fix
@szekerest can you give https://github.com/MapServer/MapServer/pull/7089 a try ?
@szekerest can you give #7089 a try ?
@rouault That seems to solve the problem in my test case rendering 1200 maps, however it is approx 3 times slower than the previous approach and the memory consumption is bigger. I'll let the client to test that in their production env.
@rouault We finally got a chance to test the mapserver proj9 fix on the client's production servers (the one that made its way into the mapserver code) and the results under load wasn't good, customers started to log support issues to complain about performance. We had to revert to using the initial fix (as per https://github.com/MapServer/MapServer/pull/7088/commits/19abdfcac45c86cc979605478c445434f7859fa8), see CPU usage with the mapserver fix, then it drops back dramatically after we revert to using the initial fix
nudge here as this was tagged for the 8.2.2 release (see Milestone) which is set to be released this Friday