Dangling File Handles?

Attached is a TestCase illustrating a problem which has popped up during
my testing. Names have been changed to protect the innocent, and to
avoid collision with possible existing extensions.

Install the package, fire up SUnit, and run
DirectoryTestCase>>#testSimpleDirectoryCreateAndDestroy. It should
green-light. Next, run #testDirectoryKill. It croaks. Look at the code.
It the same, with the exception that I touch the new directory. I store
only the pathName, no more.

Why does one test pass, and the other one fail? I even tried a garbage
collect just to flush out old references, and that didn't work.

--
Joseph Pelrine [ | ]
MetaProg GmbH
Email: [hidden email]
Web:   http://www.metaprog.com

"Inheritance was invented at 2 AM between January 5th and 6th, 1967" -
Krysten Nygaard

Joseph,

That appears to be caused by a problem with File class>>for:do:  I think
that as your #directoryNamed: method is returning directly from the do block
and therefore bypassing the cleanup of the #for:do: method, where it closes
a handle. Wrapping the complete block in an #ensure, as below,  seems to
work but I'm not sure about the #systemError returns?

Regards
    Ian

Part of File class>>for:do:

[
    [operation value: findStruct.
    lib findNextFile: handle lpFindFileData: findStruct]
        whileTrue.
    lib getLastError = ERROR_NO_MORE_FILES ifFalse: [^lib systemError]
    "(lib findClose: handle) ifFalse: [^lib systemError]"]
    ensure: [lib findClose: handle]

Ian Bartholomew wrote:

Thanks once again, Ian. That change did the trick. I was able to implement it in
my FileDirectory class, and not touch base source. I'd ask OA to take a look at
the change and maybe roll it in. One small tweak, though:

[
 [operation value: findStruct.
 lib findNextFile: handle lpFindFileData: findStruct]
   whileTrue.
 lib getLastError = ERROR_NO_MORE_FILES ifFalse: [^lib systemError]]
  ensure: [(lib findClose: handle) ifFalse: [^lib systemError]]


--
Joseph Pelrine [ | ]
MetaProg GmbH
Email: [hidden email]
Web:   http://www.metaprog.com

"Inheritance was invented at 2 AM between January 5th and 6th, 1967" -
Krysten Nygaard

"Joseph Pelrine" <[hidden email]> wrote in message
news:[hidden email]...
> Ian Bartholomew wrote:
>
> > Joseph,
> >
> > That appears to be caused by a problem with File class>>for:do:  I think
> > that as your #directoryNamed: method is returning directly from the do
block
> > and therefore bypassing the cleanup of the #for:do: method, where it
closes
> > a handle. Wrapping the complete block in an #ensure, as below,  seems to
> > work but I'm not sure about the #systemError returns?
> >
> > Regards
> >     Ian
>>...
> Thanks once again, Ian. That change did the trick. I was able to implement
it in
> my FileDirectory class, and not touch base source. I'd ask OA to take a
look at
> the change and maybe roll it in. One small tweak, though:
>
> [
>  [operation value: findStruct.
>  lib findNextFile: handle lpFindFileData: findStruct]
>    whileTrue.
>  lib getLastError = ERROR_NO_MORE_FILES ifFalse: [^lib systemError]]
>   ensure: [(lib findClose: handle) ifFalse: [^lib systemError]]
>

Thanks Ian, Joseph. #676 will make it so.

Regards

Blair

Blair McGlashan wrote:

Thanks Blair.

There's one thing I'm curious about. I spent a little extra time setting up the
problem as a SUnit test case, because I'm a firm believer in test-driven
programming. I was just curious if having the problem as a test case helped
(Ian) in debugging and finding the source of the error, or whether it was solely
construed as a waste of time. I would like to push the idea of formulating bugs
as test cases.

p.s. Ginsu just green-lighted. 120 tests all running. Now I can start working
;-)
--
Joseph Pelrine [ | ]
MetaProg GmbH
Email: [hidden email]
Web:   http://www.metaprog.com

"Inheritance was invented at 2 AM between January 5th and 6th, 1967" -
Krysten Nygaard

"Joseph Pelrine" <[hidden email]> wrote in message
news:[hidden email]...
> ...
> There's one thing I'm curious about. I spent a little extra time setting
up the
> problem as a SUnit test case, because I'm a firm believer in test-driven
> programming.

As far as we are concerned a bug report with a SUnit test attached is just
about the ideal bug report. Our first step in most cases would be to try and
create such a test anyway.

Inevitably we tend to assign well reported bugs a higher priority, so they
will get fixed sooner. The test also goes into our internal test packages so
that the bug does not resurface.

>...I was just curious if having the problem as a test case helped
> (Ian) in debugging and finding the source of the error, or whether it was
solely
> construed as a waste of time.

I don't doubt that it helped.

>...I would like to push the idea of formulating bugs
> as test cases.
>

So would we. We aren't likely to insist upon it, but we will encourage it by
asking for a test case, and addressing reports with them first.

Regards

Blair

Blair,

> As far as we are concerned a bug report with a SUnit test attached is just
> about the ideal bug report. Our first step in most cases would be to try
and
> create such a test anyway.

Ah, I mailed Joseph (cc below) saying that I thought just the opposite would
be the case - but what do I know :)  Being on the "originating" end of a bug
report obviously gives me a different perspective to those on the receiving
end. Not being a particularly keen writer of test cases can't help much
either.

Sorry Joseph!!

>        The test also goes into our internal test packages so
> that the bug does not resurface.

I didn't think of that.

> >...I was just curious if having the problem as a test case helped
> > (Ian) in debugging and finding the source of the error, or whether it
was
> solely
> > construed as a waste of time.
>
> I don't doubt that it helped.

Well, as I mention below, the first thing I did was to strip it out of the
test case so that (a) I could be sure that it wasn't something in the
testing that was interfering and (b) run it a line at a time in a workspace
to try and spot where the problem was happening.

I suppose it depends a lot on what the problem is though.

I'm off to Tesco in a minute so I shall see if the clothing dept has any
sackcloth ;-)

Ian

=~=~=~=~=

My mail to Joseph

> programming. I was just curious if having the problem as a test case
helped
> (Ian) in debugging and finding the source of the error,

Not really. The problem looked like a retained reference so the first thing
I wanted to do was strip away anything that could be holding an extra
reference - and the test framework was the first to go,  Evaluating a line
at a time from a workspace, a more controlled environment, made locating it
a lot easier as well, although in the end I traced it by the tried and
trusted technique (?) of commenting out chunks of code.

>                                or whether it was solely
> construed as a waste of time. I would like to push the idea of formulating
bugs
> as test cases.

I suppose it could help in some cases but I would have though a simple
series of workspace steps to reproduce a problem would be better in a bug
report.

Ian Bartholomew wrote:

No problem! Having survived over 2 years working with Beck, I can't write
anything without writing tests first. It's an addiction.

>
>
> >        The test also goes into our internal test packages so
> > that the bug does not resurface.

Bravo!

Having worked intensively on developing SUnit 2.1 and 3.x, and having written
literally tens of thousands of test cases in the last few years, I feel pretty
confident that SUnit doesn't interfere. Nevertheless, Ian, it *is* another
variable that one must consider during debugging, so I understand and accept
your criticism.

>
>
> I suppose it depends a lot on what the problem is though.
>
> I'm off to Tesco in a minute so I shall see if the clothing dept has any
> sackcloth ;-)
>
> Ian...

and I'm back at 22 errors in Ginsu. I deleted my Ginsu image, built a new image
and loaded my code into it, ran the tests, and got errors left and right. It
turns out that all the extensions I made to File etc. ended up landing in the
Dolphin package, which I didn't bother to check for changes before deleting the
image. Arrgh! This is why I want a default package!

--
Joseph Pelrine [ | ]
MetaProg GmbH
Email: [hidden email]
Web:   http://www.metaprog.com

"Inheritance was invented at 2 AM between January 5th and 6th, 1967" -
Krysten Nygaard