Thread unsafe hash usage in Logster

While using discourse in a multi-threaded Ruby implementation and server (TruffleRuby/Puma), errors are produced by the unsafe hash usage in the logster gem near this section:

https://github.com/discourse/logster/blob/31849a4154c6b1edb05fe6d241076329aa228359/lib/logster/logger.rb#L23-L29

3 Likes

I see, any chance you can add a Mutex there in a PR? seems like the correct approach to fixing this.

(aside @dev-managers / @jomaxro I guess for parity with other projects we should keep logster issues enabled on github? my vote is yes)

4 Likes

A post was split to a new topic: On which Discourse managed gems should we enable GitHub issues

If a Mutex is used, it should be used on both the write and read accesses for correctness.

This functionality feels like a better fit for fiber/thread-local variables though.
Is it possible to have multiple Logster::Logger instances, or is it always just one? If multiple we somehow need to make the instance’s object_id part of the key used for fiber/thread-local lookups.

1 Like

Absolutely! I think this should fix it and heavily simplify this code at the same time :+1:

2 Likes

Pretty sure there is only one, but I added some protections.

I opted out of using a define_finalizer consumers are responsible for clean up, otherwise the accounting gets very hairy.

This topic was automatically closed after 3 days. New replies are no longer allowed.