mjemmeson / Mojo-Log-JSON

Simple JSON logger for Mojo projects
Other
3 stars 0 forks source link

Possible Circular reference #7

Open stuartskelton opened 3 weeks ago

stuartskelton commented 3 weeks ago

Hi Michael,

Its been a while and I hope all is good with you. I found something that might be useful to know.

I have used this code in a project and found a circular reference in the call back in format. Passing $self into the call back can mean it enters the event loop, and in some systems the Log file handle is never closed leading to a too Too Many Files Open failure. Which in my case stopped my system from creating redis connections :(.

I have forked this in my project and copied the attributes into their in variable before giving them to the anonymous sub.

something similar to this

diff --git a/lib/Mojo/Log/JSON.pm b/lib/Mojo/Log/JSON.pm
index 54d235a..9e020ae 100644
--- a/lib/Mojo/Log/JSON.pm
+++ b/lib/Mojo/Log/JSON.pm
@@ -24,8 +24,11 @@ has default_fields => sub {
     };
 };

-has format => sub {
+has format => sub {asdadasd
     my $self = shift;
+    my $default_fields = $self->default_fields;
+    my $include_level  = $self->include_level;
+    my $codec          = $self->codec;

     return sub {
         my ( $time, $level, @lines ) = @_;
@@ -33,11 +36,11 @@ has format => sub {
         my %msg = (
             (   map {

-                    my $value = $self->default_fields->{$_};
+                    my $value = $default_fields->{$_};

                     $_ => ref $value eq 'CODE' ? $value->() : $value;

-                } keys %{ $self->default_fields }
+                } keys %{ $default_fields }
             ),

             ref $lines[0]       # data structure?
@@ -46,9 +49,9 @@ has format => sub {

         );

-        $msg{level} = $level if $self->include_level;
+        $msg{level} = $level if $include_level;

-        return $self->codec->encode( \%msg ) . "\n";
+        return $codec->encode( \%msg ) . "\n";
     };
 };
mjemmeson commented 2 weeks ago

Hi Stuart, how's things? I'd forgotten about this module tbh, we never ended up using it in the ATS team so I didn't know if it was actually useful! - instead I wrote a Log::Any proxy class (to handle the different Mojo::Log interface) and then a Log::Any::Adapter to use the Bean::Auditor library (don't know if you remember that, I think a lot of this stuff seemed to pass the Candidates team by!). Thanks for the report, I'll take a look Michael