[erlang-bugs] xmerl_scan leaks ets table

[Erik Søe Sørensen]

Hi Pascal,
You confuse me.

I did not "write the code to get this behaviour" - I used "catch" to let 
the process live on. Which is something I'd typically do if I had one 
long-lived process which were to parse a lot of XML documents, which 
makes this bug even worse.
I certainly did not write the code such in order to get a leaked table.
(Which is to say: yes, in this case I did - in order to demonstrate the 
bug...)
There is no reason to *not* destroy the table, given that its handle is 
lost.

I'm aware that if I didn't handle the exception any place, and let the 
process die, then I wouldn't have a leak problem.
However,
- the table is a resource;
- the library which creates it is responsible for cleaning it up;
- it should do so even in the case of exceptions;
- it is *quite common* to catch exceptions, especially in input 
validation related error handling such as this;
- the calling code *cannot* (without very ugly hacks such as using 
ets:all()) destroy the table on behalf of the library.

Regards,
Erik

On 06-06-2013 14:32, Pascal Chapier wrote:
> Hi Erik,
> In my opinion, when you do your test you execute the 
> xmerl_scan:string("") inside a catch from shell, the process which 
> create the table - the shell - does not die, so, there is no reason to 
> destroy the ets table, consideringing that you wrote the code to get 
> this behaviour.
>
> Best regards,
>
> Pascal
>
>
> 2013/6/6 Erik Søe Sørensen <ess@REDACTED <mailto:ess@REDACTED>>
>
>     Hi there -
>     When parsing a string, xmerl_scan creates an ets table which it
>     doesn't delete again in the case of exceptions.
>
>     Demo:
>
>         Erlang R16B (erts-5.10.1) [source] [64-bit] [smp:8:8]
>         [async-threads:10] [hipe] [kernel-poll:false]
>
>         Eshell V5.10.1  (abort with ^G)
>         1> length(ets:all()).
>         16
>         2>
>         2> catch xmerl_scan:string("").
>         3900- fatal: expected_element_start_tag
>         {'EXIT',
>             {fatal,
>                 {expected_element_start_tag,
>                     {file,file_name_unknown},
>                     {line,1},
>                     {col,1}}}}
>         3> length(ets:all()).
>         17
>
>     (I just re-discovered this bug; I apologize for having neglected
>     to report it earlier, considering that I've noticed it before some
>     years ago.)
>
>     From a brief inspection at the code, I think the place to ensure
>     table deletion would be to add a "try...after" in scan_document.
>     Alternatively, it could be done in int_string/4 and
>     int_string_decl/4, where the cleanup can mirror the calls to
>     initial_state0()->initial_state() where the ets table is created.
>
>     Regards,
>     Erik Søe Sørensen
>
>     -- 
>     Mobile: + 45 26 36 17 55 | Skype: eriksoesorensen | Twitter: @eriksoe
>     Trifork A/S  |  Margrethepladsen 4  |  DK-8000 Aarhus C |
>     www.trifork.com <http://www.trifork.com/>
>
>     _______________________________________________
>     erlang-bugs mailing list
>     erlang-bugs@REDACTED <mailto:erlang-bugs@REDACTED>
>     http://erlang.org/mailman/listinfo/erlang-bugs
>
>


-- 
Mobile: + 45 26 36 17 55 | Skype: eriksoesorensen | Twitter: @eriksoe
Trifork A/S  |  Margrethepladsen 4  |  DK-8000 Aarhus C | 
www.trifork.com <http://www.trifork.com/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-bugs/attachments/20130607/cb000cac/attachment.htm>