Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#1774 error in timeout destroys token

obsolete: 8.4a4
closed-duplicate
Pat Thoyts
5
2008-02-22
2002-02-05
Don Porter
No

The timeout option for http::geturl is
implemented by a timer event that calls

http::reset $token timeout

If there's an error returned by the command
specified by the -command option, then
http::reset will unset the state array and
raise an error that will be processed as
a background error.

All this processing takes place in an event
loop, probably within [http::wait $token].
Given the above, it appears that callers
of [http::wait $token] need to be able to
handle the non-existence of $token upon
its return. Also, [http::wait] itself needs to
be written to deal with the non-existence
of $token when its call to [vwait] returns.

Something needs fixing around lines 358 and 840
of the current http.tcl.

Discussion

  • Don Porter
    Don Porter
    2002-02-28

    Logged In: YES
    user_id=80530

    Would be good to fix this for the release of http 2.4.2
    with Tcl 8.4a4. Is there an historical reason for the
    unset of the state array in [reset] ? It seems to be
    the cause of the trouble. Wouldn't we rather keep the
    state array around with a status of "error"?

    The log indicates that "sandeep" added the unset to
    [reset] in Revision 1.28 of http.tcl to "fix two
    potential memory leaks". Anyone have more detailed
    recollection about this?

     
  • Jeffrey Hobbs
    Jeffrey Hobbs
    2002-02-28

    Logged In: YES
    user_id=72656

    IIRC, if you don't reset when that happens, you otherwise
    leave around that array (because an http::cleanup is not
    expected). That is the mem leak - one at the Tcl level by
    leaving http state arrays around.

     
  • Jeffrey Hobbs
    Jeffrey Hobbs
    2002-02-28

    • assigned_to: hobbs --> dgp
     
  • Don Porter
    Don Porter
    2002-03-01

    Logged In: YES
    user_id=80530

    I looked over this again, and it's really unclear to
    me whose reponsibility it should be to do the cleanup.
    [geturl] returns a token, and the public command [cleanup]
    is available, so that suggests to me it should be the
    caller's job to cleanup tokens, but in some parts the
    http package appears to act as its own caller.

    Unfortunately, looking through the code did not help
    much, since there are several related commands [reset],
    [Finish], [cleanup], etc. without a real clear distinction
    about what job each one has to perform. A good auditing
    and refactoring could be helpful here.

     
  • Jeffrey Hobbs
    Jeffrey Hobbs
    2004-05-25

    Logged In: YES
    user_id=72656

    Is this related to 705956 ?

     
  • Don Porter
    Don Porter
    2004-05-26

    Logged In: YES
    user_id=80530

    only in the general sense
    that the cleanup logic of
    the http package is
    an utter mess, and both
    these problems arise from
    that.

    However, the specific problem
    in this report is that the state
    array is unset by [http::reset],
    so whenever both the -timeout
    and -command options are in
    use, and the timeout triggers
    and the -command callback
    returns an error, then the [vwait]
    within [http::wait] will return
    with the state array unset.
    This will cause the command

    return $state(status)

    on line 848 to raise an error
    about being unable to read
    the variable that's been unset.

    Any code not calling [http::wait]
    but relying on another event loop
    may have similar problems, if it
    is not prepared for the possibility
    that the state array may be unset.

    Bug 705956 doesn't appear
    to be about having the state
    array unset at all. The -timeout
    option is not involved. In contrast,
    it assumes the state array still
    exists, and the value of $state(status)
    is "error" and the issue is whether
    some cleanup logic based on that
    value has been inverted.

     
  • Don Porter
    Don Porter
    2007-11-15

    • assigned_to: dgp --> patthoyts
     
  • Don Porter
    Don Porter
    2007-11-27

    Logged In: YES
    user_id=80530
    Originator: YES

    patthoyts, had any chance to look at this?

     
  • Pat Thoyts
    Pat Thoyts
    2008-02-22

    • status: open --> closed-duplicate
     
  • Pat Thoyts
    Pat Thoyts
    2008-02-22

    Logged In: YES
    user_id=202636
    Originator: NO

    Same as #1818565