أحمد المحمودي <[EMAIL PROTECTED]> writes:

>   ELinks 0.13 (20080915 snapshot) crashes when a download finishes, 
>   that is in the case that this download was started from a window that 
>   is closed before the download finishes. Here are the steps to 
>   reproduce the bug:
>   1] in tty1 for example, start elinks and go to the elinks download 
>      page.
>   2] in tty2 for example, start elinks (do not use --no-connect) and go 
>      to another page.
>   3] start downloading one of the snapshots in the elinks started in 
>      tty1, then close elinks that is running in tty1.
>   4] The download will continue since elinks is running in tty2, yet 
>      when the download is over, elinks will crash.

ELinks 0.11.5 crashes too.  With valgrind, it's easiest to start
the download on the terminal on which ELinks was started last.

==23545== Invalid read of size 4
==23545==    at 0x806B11C: intl_set_charset (libintl.h:64)
==23545==    by 0x806B0FA: _ (libintl.h:90)
==23545==    by 0x806B19A: msg_text (msgbox.c:127)
==23545==    by 0x80FF617: download_data_store (download.c:361)
==23545==    by 0x80FF93E: download_data (download.c:421)
==23545==    by 0x80CCF56: notify_connection_callbacks (connection.c:427)
==23545==    by 0x80CD035: done_connection (connection.c:444)
==23545==    by 0x80CD77F: add_keepalive_connection (connection.c:602)
==23545==    by 0x80F5472: http_end_request (http.c:482)
==23545==    by 0x80F6DBE: read_http_data_done (http.c:1140)
==23545==    by 0x80F733C: read_http_data (http.c:1331)
==23545==    by 0x80D1DD2: read_select (socket.c:879)
==23545==  Address 0x45218E0 is 72 bytes inside a block of size 397 free'd
==23545==    at 0x402210F: free (vg_replace_malloc.c:233)
==23545==    by 0x81171D3: debug_mem_free (memdebug.c:468)
==23545==    by 0x810F93D: destroy_terminal (terminal.c:150)
==23545==    by 0x80C794D: check_bottom_halves (select.c:115)
==23545==    by 0x80C8032: select_loop (select.c:284)
==23545==    by 0x80C735E: main (main.c:357)

I guess we should make destroy_terminal() search for download.term
pointers to that terminal, and reset them to NULL.

struct download is used in these structures:
- struct type_query: Each of these is associated with a dialog
  box, so there should be no such structures left when the
  terminal is being destroyed.
- struct file_download: These are in the global list "downloads".
- struct location: Can these be a problem?
- struct session: Each of these is associated with a tab window,
  so there should be no such structures left when the terminal is
  being destroyed.
- struct file_to_load: Can these be a problem?
- struct bittorrent_fetcher: It seems download.term is always
  NULL here.

And then there are also instances of struct download that are not
embedded in any larger structure, e.g. in scripting/python/load.c
and viewer/dump/dump.c.

Some ideas for fixing this:

(a) Make download_data_store() walk the "terminals" list and
    check that download.term is in the list.  I don't like this
    because the C standard says the value of a pointer to freed
    memory is indeterminate and comparing it to anything already
    results in undefined behaviour.  In practice, it could make
    ELinks report completion of downloads to the wrong terminal.

(b) Assume that only struct file_download can really cause a
    crash.  Make destroy_terminal() walk the "downloads" list.
    Also, #ifdef CONFIG_DEBUG, make download_data_store() assert
    that download.term is in the "terminals" list.

(c) Add a separate list to which _every_ struct download is
    added.  Define init_download and done_download functions.
    This would require changes in a number of places, I'm afraid.
    However it would also make enhancement 435 easier.

(d) As in (c) but also define a linked list for each terminal,
    listing the struct download instances that point to it.
    All updates of download.term must go through a new function
    set_download_term(), which maintains these lists.  This way,
    destroy_terminal() would not have to look at downloads that
    do not use the terminal.  However, this scheme would bloat
    struct download and probably not speed up destroy_terminal()
    noticeably.

Overall I think we should do (b) now in 0.11.5.GIT and 0.12pre2.GIT,
and (c) in 0.13.GIT after the 0.12.0 release.

Attachment: pgpUI9FfYpXsc.pgp
Description: PGP signature

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Reply via email to