[Tcl-bugs] [ tcl-Bugs-3024359 ] Crash in Tcl_FSGetFileSystemForPath

[SourceForge.net <no...@so...>]

Bugs item #3024359, was opened at 2010-07-02 06:02
Message generated for change (Comment added) made by dgp
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3024359&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 37. File System
Group: obsolete: 8.5.8
>Status: Pending
>Resolution: Fixed
Priority: 8
Private: No
Submitted By: Neil Hampton (neilhampton)
Assigned to: Don Porter (dgp)
Summary: Crash in Tcl_FSGetFileSystemForPath

Initial Comment:
We have a multi-threaded tclkit application that runs several scripts on different threads and it crashes in Tcl_FSGetFileSystemForPath on the line "fsRecPtr = fsRecPtr->nextPtr".

From investigation, it appears that 'theFilesystemEpoch' is being updated within the loop and the threads file system list is being re-cached leaving fsRecPtr pointing at freed memory.  It is very difficult to track, but it appears that Tcl_FSMountsChanged is being called from within Tcl_FSGetFileSystemForPath.  The path being accessed is the 'wrapped' init.tcl.

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

>Comment By: Don Porter (dgp)
Date: 2012-06-25 09:44

Message:
Fix merged to trunk as well.

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

Comment By: Don Porter (dgp)
Date: 2012-06-25 08:21

Message:
Fix committed to core-8-5-branch for release in 8.5.12.

Confirmation that it fixes the reported bug would be welcome.

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

Comment By: Don Porter (dgp)
Date: 2012-06-22 09:45

Message:
Revised the code to restore the crashing.  The bug is still
present on 8-5-branch, but was able to hide by getting lucky
with memory allocation patterns.  Now it cannot hide.  Bugs
that don't hide are easier to fix.

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

Comment By: Don Porter (dgp)
Date: 2012-06-21 12:12

Message:
The crashing is much less frequent though, which
suggests that the FsGetFirstFilesystem() call in the
(now deleted) helper routine TclFsInternalToNormalized()
was the one most frequently causing a bump in the
per-thread filesystemEpoch value at a time that could cause
trouble.

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

Comment By: Don Porter (dgp)
Date: 2012-06-21 12:00

Message:
Aha!  Strike that last comment.  Still seeing
crashes on 8.5 branch.  Developing fix is
still on the bug-3024359 branch.

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

Comment By: Don Porter (dgp)
Date: 2012-06-21 11:54

Message:

Some simplifications of the internals of fs path values
just committed to the 8.5 and trunk branches has the
effect of also making the posted test pass.

I have my doubts these changes are a complete fix for
the issue of managing the mount epoch properly (still
pursuing that on the bug-3024359 branch) but they may
be sufficient to repair the problems observed by the
original poster.

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

Comment By: Don Porter (dgp)
Date: 2012-06-18 13:37

Message:

Next draft fix committed.  This one appears to solve
the problem, at least as far as that test demonstrates.

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

Comment By: Don Porter (dgp)
Date: 2012-06-18 12:40

Message:

This seems to be a controlled way to test
for the bug.  Doesn't easily convert to a test
for the test suite.

package require Thread
thread::create {while 1 {set env(HOME) /home/user[incr i]}}
while 1 {file system foo}


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

Comment By: Don Porter (dgp)
Date: 2012-06-12 06:46

Message:

Second draft committed, but it's now clear these patches
are broken.  Although they preserve the FilesystemRecord
linked lists while they are in use, avoiding the reported crashes,
the result is that when a search of these preserved records
succeeds, the incorrect epoch value is recorded, so that a
record retrieved from an outdated epoch is stored as if it
were current.

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

Comment By: Don Porter (dgp)
Date: 2012-06-11 15:26

Message:

First draft fix committed to branch bug-3024359.  Needs tests and testing.

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

Comment By: Don Porter (dgp)
Date: 2012-06-11 09:41

Message:
The issue appears to be care needed by callers of
FsGetFirstFilesystem().  Any caller that keeps and
makes use of the value it returns needs to take care
not to continue using that value whenever another
call to FsGetFirstFilesystem() might be made in the
same thread.  Since TclFSEnsureEpochOk() does
just that, it's clear that Tcl_FsGetFileSystemForPath()
has the potential for this kind of trouble.  The need to
make both those calls and make sure they both report
results on the same epoch is a bit of a puzzle.

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

Comment By: Don Porter (dgp)
Date: 2012-05-29 06:40

Message:
This is reported against 8.5.8.  Has it been reconfirmed
with either 8.5.11 release, or current state of core-8-5-branch?

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3024359&group_id=10894