list_to_atom use in kernel/error_logger.erl

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

list_to_atom use in kernel/error_logger.erl

Serge Aleynikov
I was browsing through the kernel/error_logger.erl and the following
code caught my attention.  Wouldn't the call list_to_atom/1 eventially
lead to exhaustion of the atom table?

----kernel/error_logger.erl (line: 288)-------------------
display2(Tag,F,A) ->
     erlang:display({error_logger,Tag, nice(F), nice(A)}).

nice(X) when tuple(X) ->
     nice_tuple(X);

nice([]) -> [];
nice(L) when list(L) ->
     case is_string(L) of
     true ->
         list_to_atom(L);
     false ->
         [H|T] = L,
         [nice(H) | nice(T)]
     end;
nice(X) -> X.
----------------------------------------------------------

Why does a list need to be converted to atom here?

Serge
Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Matthias Lang
Hi,

Short answer: It's not a problem, the offending code is only
              executed during the Erlang boot.

Long answer: This seemed like a jolly good opportunity to make the
emulator crash and burn. Alas, it's harder than hoped.

The manpage for the error_logger hints (see swap_handler) that the
error_handler starts up in a 'primitive' state, before being moved to
'normal' mode. The code you posted is only reachable from that
primitive state, so there's not much scope for logging a bunch of junk
with it.

If you insist on clogging the works, you can use the undocumented export
error_loger:simple_logger/1 with an integer argument.

Matthias

--------------------

Serge Aleynikov writes:
 > I was browsing through the kernel/error_logger.erl and the following
 > code caught my attention.  Wouldn't the call list_to_atom/1 eventially
 > lead to exhaustion of the atom table?
 >
 > ----kernel/error_logger.erl (line: 288)-------------------
 > display2(Tag,F,A) ->
 >      erlang:display({error_logger,Tag, nice(F), nice(A)}).
 >
 > nice(X) when tuple(X) ->
 >      nice_tuple(X);
 >
 > nice([]) -> [];
 > nice(L) when list(L) ->
 >      case is_string(L) of
 >      true ->
 >          list_to_atom(L);
 >      false ->
 >          [H|T] = L,
 >          [nice(H) | nice(T)]
 >      end;
 > nice(X) -> X.
 > ----------------------------------------------------------
 >
 > Why does a list need to be converted to atom here?
 >
 > Serge
Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Serge Aleynikov
Matthias,

Thank you for clarification.  The reason I looked into the
error_logger's code was indeed related to the ugly format of the
displayed message upon an unsuccessful startup with a -boot option.

Though I still don't understand the need for list_to_atom(L) there.  Why
not just return the list L?  It does work either way.

Serge

Matthias Lang wrote:

> Hi,
>
> Short answer: It's not a problem, the offending code is only
>               executed during the Erlang boot.
>
> Long answer: This seemed like a jolly good opportunity to make the
> emulator crash and burn. Alas, it's harder than hoped.
>
> The manpage for the error_logger hints (see swap_handler) that the
> error_handler starts up in a 'primitive' state, before being moved to
> 'normal' mode. The code you posted is only reachable from that
> primitive state, so there's not much scope for logging a bunch of junk
> with it.
>
> If you insist on clogging the works, you can use the undocumented export
> error_loger:simple_logger/1 with an integer argument.
>
> Matthias
>
> --------------------
>
> Serge Aleynikov writes:
>  > I was browsing through the kernel/error_logger.erl and the following
>  > code caught my attention.  Wouldn't the call list_to_atom/1 eventially
>  > lead to exhaustion of the atom table?
>  >
>  > ----kernel/error_logger.erl (line: 288)-------------------
>  > display2(Tag,F,A) ->
>  >      erlang:display({error_logger,Tag, nice(F), nice(A)}).
>  >
>  > nice(X) when tuple(X) ->
>  >      nice_tuple(X);
>  >
>  > nice([]) -> [];
>  > nice(L) when list(L) ->
>  >      case is_string(L) of
>  >      true ->
>  >          list_to_atom(L);
>  >      false ->
>  >          [H|T] = L,
>  >          [nice(H) | nice(T)]
>  >      end;
>  > nice(X) -> X.
>  > ----------------------------------------------------------
>  >
>  > Why does a list need to be converted to atom here?
>  >
>  > Serge
>

--
Serge Aleynikov
R&D Telecom, IDT Corp.
Tel: (973) 438-3436
Fax: (973) 438-1464
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Gunilla Arendt
Serge Aleynikov wrote:
>
> Thank you for clarification.  The reason I looked into the
> error_logger's code was indeed related to the ugly format of the
> displayed message upon an unsuccessful startup with a -boot option.
>
> Though I still don't understand the need for list_to_atom(L) there.  Why
> not just return the list L?  It does work either way.
>

I haven't tried to verify this, but my guess would be that someone tried
to make the output from error_logger more readable in the case where
the Erlang run-time system fails to start.

For example, if an application in the boot script fails to start, you
will see something like this:

$ erl
{error_logger,{{2006,2,2},{14,11,52}},'Protocol: ~p: register error:
~p~n',[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
...

which is of course somewhat more readable than

{error_logger,{{2006,2,2},{14,11,52}},[80,114,111,116,111,99,111,108,58,32,126,1
12,58,32,114,101,103,105,115,116,101,114,32,101,114,114,111,114,58,32,126,112,12
6,110],[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
...

/ Gunilla


Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Serge Aleynikov
I believe what you are referring to is actually a small issue with the
application_controller rather than error_logger.  If the following patch
is applied to the application_controller then the logging problem at
system startup that you indicated will be more readable and you could
remove the list_to_atom(L) in the error_logger.erl.

--- kernel-2.10.12/src/application_controller.erl Fri Jan 27 17:17:31 2006
+++ lib/kernel/src/application_controller.erl Thu Feb  2 10:12:01 2006
@@ -1932,6 +1932,6 @@
               true ->
                   Term;
               false ->
-                 lists:flatten(io_lib:write(Term))
+                 lists:flatten(io_lib:format("~p", [Term]))
           end,
      lists:sublist(Str, 199).

Regards,

Serge

Gunilla Arendt wrote:

> Serge Aleynikov wrote:
>
>>
>> Thank you for clarification.  The reason I looked into the
>> error_logger's code was indeed related to the ugly format of the
>> displayed message upon an unsuccessful startup with a -boot option.
>>
>> Though I still don't understand the need for list_to_atom(L) there.  
>> Why not just return the list L?  It does work either way.
>>
>
> I haven't tried to verify this, but my guess would be that someone tried
> to make the output from error_logger more readable in the case where
> the Erlang run-time system fails to start.
>
> For example, if an application in the boot script fails to start, you
> will see something like this:
>
> $ erl
> {error_logger,{{2006,2,2},{14,11,52}},'Protocol: ~p: register error:
> ~p~n',[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>
> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>
> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
> ...
>
> which is of course somewhat more readable than
>
> {error_logger,{{2006,2,2},{14,11,52}},[80,114,111,116,111,99,111,108,58,32,126,1
>
> 12,58,32,114,101,103,105,115,116,101,114,32,101,114,114,111,114,58,32,126,112,12
>
> 6,110],[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>
> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>
> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
> ...
>
> / Gunilla
>
>
>

--
Serge Aleynikov
R&D Telecom, IDT Corp.
Tel: (973) 438-3436
Fax: (973) 438-1464
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Gunilla Arendt
Clarification: I should have added that I think list_to_atom/1 is in
error_logger for *historical reasons*, i.e. my guess is that it's a
remnant from a time when erlang:display/1 displayed strings as list of
ascii codes. Nowadays, erlang:display/1 can handle strings as well as
atoms and the call to list_to_atom/1 is indeed superfluous.

That said, I would like to point out that messages to error_logger could
be sent from any process, so it is (was) not an application_controller
issue. It is true that application_controller sends lists to
error_logger, but so could any other process. Also, the code snippet you
refer to actually has to do with the exit reason of the
application_controller process, not an error report.

/ Gunilla

Serge Aleynikov wrote:

> I believe what you are referring to is actually a small issue with the
> application_controller rather than error_logger.  If the following patch
> is applied to the application_controller then the logging problem at
> system startup that you indicated will be more readable and you could
> remove the list_to_atom(L) in the error_logger.erl.
>
> --- kernel-2.10.12/src/application_controller.erl Fri Jan 27 17:17:31 2006
> +++ lib/kernel/src/application_controller.erl Thu Feb  2 10:12:01 2006
> @@ -1932,6 +1932,6 @@
>               true ->
>                   Term;
>               false ->
> -                 lists:flatten(io_lib:write(Term))
> +                 lists:flatten(io_lib:format("~p", [Term]))
>           end,
>      lists:sublist(Str, 199).
>
> Regards,
>
> Serge
>
> Gunilla Arendt wrote:
>
>> Serge Aleynikov wrote:
>>
>>>
>>> Thank you for clarification.  The reason I looked into the
>>> error_logger's code was indeed related to the ugly format of the
>>> displayed message upon an unsuccessful startup with a -boot option.
>>>
>>> Though I still don't understand the need for list_to_atom(L) there.  
>>> Why not just return the list L?  It does work either way.
>>>
>>
>> I haven't tried to verify this, but my guess would be that someone tried
>> to make the output from error_logger more readable in the case where
>> the Erlang run-time system fails to start.
>>
>> For example, if an application in the boot script fails to start, you
>> will see something like this:
>>
>> $ erl
>> {error_logger,{{2006,2,2},{14,11,52}},'Protocol: ~p: register error:
>> ~p~n',[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>>
>> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>>
>> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
>> ...
>>
>> which is of course somewhat more readable than
>>
>> {error_logger,{{2006,2,2},{14,11,52}},[80,114,111,116,111,99,111,108,58,32,126,1
>>
>> 12,58,32,114,101,103,105,115,116,101,114,32,101,114,114,111,114,58,32,126,112,12
>>
>> 6,110],[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>>
>> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>>
>> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
>> ...
>>
>> / Gunilla
>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: list_to_atom use in kernel/error_logger.erl

Serge Aleynikov
Gunilla,

Gunilla Arendt wrote:
> Clarification: I should have added that I think list_to_atom/1 is in
> error_logger for *historical reasons*, i.e. my guess is that it's a
> remnant from a time when erlang:display/1 displayed strings as list of
> ascii codes. Nowadays, erlang:display/1 can handle strings as well as
> atoms and the call to list_to_atom/1 is indeed superfluous.

Thanks for clarification!  I agree with your thinking, however, since
this simple error_logger is replaced with a standard error handler at
startup, I assume it's logging is only essential for the startup
functionality of the application_controller, which is a pretty
imprortant place in troubleshooting startup issues.

> That said, I would like to point out that messages to error_logger could
> be sent from any process, so it is (was) not an application_controller
> issue. It is true that application_controller sends lists to
> error_logger, but so could any other process. Also, the code snippet you
> refer to actually has to do with the exit reason of the
> application_controller process, not an error report.

As far as I understand you cannot use io_lib's functions for formatting
error reports in that error_logger as they lead to a deadlock.  Given
so, I believe, it is extremely useful to see the failure reason printed
  readably.

On the other hand, perheps when you are exposed to Erlang for quite a
while, the difference between

       [80,114,111,116,111,99,111,108] and "Protocol"

starts to evaporate, and you begin to think that the green flickering
symbols on computer terminals in the "Matrix" movie do resemble people
behind them...    ;-)

Serge

P.S. I did apply the patch to my application_controller.erl, and I am
quite happy now as I can read the starup failure reasons without opening
erl and copying/pasting the failure printouts in the shell like this:

(a@dev_serge2)2> [80,114,111,116,111,99,111,108].
"Protocol"

> Serge Aleynikov wrote:
>
>> I believe what you are referring to is actually a small issue with the
>> application_controller rather than error_logger.  If the following
>> patch is applied to the application_controller then the logging
>> problem at system startup that you indicated will be more readable and
>> you could remove the list_to_atom(L) in the error_logger.erl.
>>
>> --- kernel-2.10.12/src/application_controller.erl Fri Jan 27 17:17:31
>> 2006
>> +++ lib/kernel/src/application_controller.erl Thu Feb  2 10:12:01 2006
>> @@ -1932,6 +1932,6 @@
>>               true ->
>>                   Term;
>>               false ->
>> -                 lists:flatten(io_lib:write(Term))
>> +                 lists:flatten(io_lib:format("~p", [Term]))
>>           end,
>>      lists:sublist(Str, 199).
>>
>> Regards,
>>
>> Serge
>>
>> Gunilla Arendt wrote:
>>
>>> Serge Aleynikov wrote:
>>>
>>>>
>>>> Thank you for clarification.  The reason I looked into the
>>>> error_logger's code was indeed related to the ugly format of the
>>>> displayed message upon an unsuccessful startup with a -boot option.
>>>>
>>>> Though I still don't understand the need for list_to_atom(L) there.  
>>>> Why not just return the list L?  It does work either way.
>>>>
>>>
>>> I haven't tried to verify this, but my guess would be that someone tried
>>> to make the output from error_logger more readable in the case where
>>> the Erlang run-time system fails to start.
>>>
>>> For example, if an application in the boot script fails to start, you
>>> will see something like this:
>>>
>>> $ erl
>>> {error_logger,{{2006,2,2},{14,11,52}},'Protocol: ~p: register error:
>>> ~p~n',[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>>>
>>> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>>>
>>> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
>>> ...
>>>
>>> which is of course somewhat more readable than
>>>
>>> {error_logger,{{2006,2,2},{14,11,52}},[80,114,111,116,111,99,111,108,58,32,126,1
>>>
>>> 12,58,32,114,101,103,105,115,116,101,114,32,101,114,114,111,114,58,32,126,112,12
>>>
>>> 6,110],[inet_tcp,{{badmatch,{error,duplicate_name}},[{inet_tcp_dist,listen,1},{ne
>>>
>>> t_kernel,start_protos,4},{net_kernel,start_protos,3},{net_kernel,init_node,2},{n
>>>
>>> et_kernel,init,1},{gen_server,init_it,6},{proc_lib,init_p,5}]}]}
>>> ...
>>>
>>> / Gunilla
>>>
>>>
>>>
>>
>
>