#4208 Seek causes IO channel to be shared between all interps

obsolete: 8.5.5
closed-fixed
9
2008-12-11
2008-12-08
No

I noticed that 'seek 0' on a file handle makes the handle accessible in all interps. I can't find any evidence in the docs that this is the intended behavior.

Example: The following script will dump 2 x four bytes from /dev/random:

interp create foo
interp create bar

set f [open /dev/random rb]

interp eval foo {
proc bytes {fh} {
binary scan [read $fh 4] H* hex
puts $hex
close $fh
}
}

interp eval bar {
proc bytes {fh} {
binary scan [read $fh 4] H* hex
puts $hex
close $fh
}
}

seek $f 0
interp eval foo [list bytes $f]
interp eval bar [list bytes $f]
close $f

Without the seek command it reports an error as expected:
can not find channel named "file5"
while executing
"read $fh 4"
(procedure "bytes" line 2)
invoked from within
"bytes file5"
invoked from within
"interp eval foo [list byte $f]"

SuSE linux 11.0
Kernel 2.6.25.18-0.2-pae
Tcl 8.5.5

Discussion

  • Donal K. Fellows

    • labels: 105661 --> 25. Channel System
    • priority: 5 --> 9
    • assigned_to: hobbs --> andreas_kupries
     
  • Donal K. Fellows

    Diagnosis: leakage of file handle through Tcl_Obj internal rep. Problem is not [seek] itself, but rather Tcl_GetChannelFromObj.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    Ok, coming to this I see the problem in 8.5, but not 8.6/Head. As I also saw a commit by dkf to tclIO.c I am assuming right now that the 'Refactoring tclIO.c' commit was (intended as) a fix for this. Meaning that I 'just' have to back port it. Is that correct, Donal ?

     
  • Jeffrey Hobbs

    Jeffrey Hobbs - 2008-12-09

    Donal's changes do not look like they intentionally changed any behavior. I'm skeptical that TclGetChannelFromObj is the source of the issue as it layers over Tcl_GetChannel, which maintains a table in the assocData of a single interp, and uses Tcl_Objs, which should not be shared. Am I missing something?

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    Note 1: There is no Tcl_GetChannelFromObj(). Only a TclGetChannelFromObj(). I.e. internal, not public.
    Note 2: Trying to trace where the fix is in all the unrelated changes is ... not fun. There do not seem to be any changes to fix it.

    Tracing the TclGetChannelFromObj a bit it seems that in 8.6 something invalidates the intrep after the seek, before the 'puts' in interp foo is reached. So far no idea what.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    @jeffh: The intrep of tclChannelType is the statePtr (ChannelState) of a channel, without an association to an interp. This means a Tcl_Obj with vlaid intrep can carry a channel into an interp which actually doesn't know, and it will be properly used by direct access to its state, without looking it up anywhere.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    Ok, I was possibly not too clear.
    SetChannelFromAny (SCFA) uses Tcl_GetChannel.
    However if and only if the intrep is not already tclChannelType.
    If the intrep is properly tclChannelType it pulls the ChannelState*
    directly out of the intrep, and now we have direct access to a channel
    which may not be known to the interpreter we are in, because the intrep
    was set in a different interpreter.
    So, even with a properly typed intrep we have to recheck that the channel
    we got is actually known in the current interp, and the same.

    Best would be to not only store channelstate*, but also interp* of the interp which set the intrep. The moment we are in a different interp we invalidate and recompute the intrep. sort of like cmd resolution intreps.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    Proposed fix, extended intrep and revalidation when changing interpreters.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    Added patch containing my proposed fix.
    dkf, schelte, please review.

     
  • Donal K. Fellows

    sounds like a reasonable fix. You also probably need to check an epoch counter or something like that so that if the channel is deleted and reallocated, that can be handled too...

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-09

    I do not believe that an epoch is required.
    SCFA checks the flags of the preserved channelState for CHANNEL_CLOSED.
    If that is true it already invalidated and refetched the intrep.
    This should handle the close/reopen case.

     
  • Donal K. Fellows

    If interpreter boundaries are the only concern (the "preservation within an interp" issue is a great thing to have a test case on in any case) then the patch is good. Needs a test, but that can probably be adapted from the bug report.

     
  • Don Porter

    Don Porter - 2008-12-10

    comments on the chat indicated the
    solution here involved having the
    intrep of a Tcl_Obj set a Tcl_Preserve
    on a struct, with a matching Tcl_Release
    when the intrep is shimmered away.

    That's fine as a stopgap, but if at
    all possible, the need to do that
    means the preserved struct really
    needs its own refCount field.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-10

    @dgp: A bit of clarification.

    The functions of tclChannelType do indeed Tcl_Preserve/Release ChannelState pointers. This is however not because of my fix. They did that even before I put my hands into this particular pie. As such it is an integral part of this TclObjTyp I was not really willing to change for my fix. What I did was to extend this scheme to the Interp pointer I added to the intrep. It might actually not be necessary, as the Interp pointer is only copied and compared, never de-referenced, contrary to the ChannelState.

    I should further note that ChannelState structures actually have a refCount. This however is used to count how many interpreters have the the channel Tcl_RegisterChannel()'ed in them, and a refCount > 0 prevents fully closing the channel. I do not believe that it is a good idea to use this refCount to count how many Tcl_Obj refer to this channel. As it means that a 'close $f' will never really close the channel, but only at some arbitrary time later when the last intrep goes away. Which actually may be never during the lifetime of an interp.

    I will see that I can get a test done for 8.5 and 8.6 in the next days, and then check if may Preseve/Release of the Interp* was overkill. Likely yes. If so I will remove that part of the patch.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-11

    Updated fix for 8.5, test case, simplified interp* handling

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-11

    Updated fix for 8.6, test case, simplified interp* handling

     
  • Schelte Bron

    Schelte Bron - 2008-12-11

    I don't know enough of the internals of Tcl to be able to give a theoretical opinion about the patch. I have confirmed that it works in practice with the application where I found the issue.

    One additional remark: I found that this bug was not present in Tcl 8.5.2. It must have been introduced in Tcl 8.5.3 or 8.5.4.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-11

    Applied patches to both 8.5 and head.

     
  • Andreas Kupries

    Andreas Kupries - 2008-12-11
    • status: open --> closed-fixed
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks