Loggable now works with frozen classes and objects.

[eviljoel]

Issue

233

Changelog

• Loggable now works with frozen classes and objects.

Description of changes

Changed the Loggable module to not set any object or class instance variables making Loggable compatible with frozen classes and objects. I'm using a constant Hash in the Loggable module to cache the Loggers so a new one is not created on every use. Changes should be backwards compatible with the current Loggable module and nearly as performant.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[keithrbennett]

Another approach would be to initialize the class instance variable (lazily or not) with a logger and just call self.class.logger in the logger instance method. This would eliminate the hash lookup on every logger call.

I realize this eliminates support for frozen classes though. How important is that support?

[eviljoel]

Another approach would be to initialize the class instance variable (lazily or not) with a logger and just call self.class.logger in the logger instance method. This would eliminate the hash lookup on every logger call.

I realize this eliminates support for frozen classes though. How important is that support?

We could choose to ignore frozen classes and just change the behavior the instance logger method, but I thought it would be a more complete solution to address both frozen objects and frozen classes.

I'll be forthcoming and admit there is a disadvantage to supporting frozen classes in this way. If a program dynamically creates lots of classes and then deletes those classes using remove_const, the Logger for those classes will not be removed. This would result in a memory leak. I tried to get around this problem using ObjectSpace.define_finalizer but ran into two problems:

• Finalizers cannot be applied to frozen objects. (In this case, a Class object.)
• If a finalizer is declared when the object being finalized is in the call stack, then a reference to that object is maintained for the remainder of the program. This eliminates the usefulness of a finalizer in this context.

A third option is to only use LOGGABLE_LOGGERS for frozen classes. That way a memory leak is even less likely but there would be some additional overhead in the class logger method.

Anyway, we have several options here. Ultimately, the choice is up to Reid.

[keithrbennett]

Although I find the proposed code change impressive, my opinion is that it would be better to support instances only, cleanly and simply, rather than adding the complexity and risk associated with the class and instance approach.

[reidmorrison]

This change is not thread safe. It is bad practice not to freeze constant values. Semantic Logger is designed to be lazy loaded. I would suggest you look at alternative logging implementations when using frozen objects. Other options are to load the logger instance inside your initializer, or not to use the helper mixin, and instead define the logging methods in your class, as you would do if you were using the Ruby built in Logger class.