ExternalPipe atEnd test

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

ExternalPipe atEnd test

K K Subbu
Hi,

I stumbled upon the following method in OSProcess-Base package:

ExternalPipe#atEnd

  ^writer closed and: [self peek == nil]

Shouldn't the test be reversed? In a pipe, the reader's peek buffer
could be holding an object while the writer has closed its end.

Regards .. Subbu

Reply | Threaded
Open this post in threaded view
|

Re: ExternalPipe atEnd test

Alistair Grant
If you reverse the test there is a window where data may be written to the pipe between the two tests, and the data is subsequently lost.

Cheers,
Alistair
(on phone)

On Thu., 12 Apr. 2018, 09:04 K K Subbu, <[hidden email]> wrote:
Hi,

I stumbled upon the following method in OSProcess-Base package:

ExternalPipe#atEnd

  ^writer closed and: [self peek == nil]

Shouldn't the test be reversed? In a pipe, the reader's peek buffer
could be holding an object while the writer has closed its end.

Regards .. Subbu



Reply | Threaded
Open this post in threaded view
|

Re: ExternalPipe atEnd test

David T. Lewis
In reply to this post by K K Subbu
On Thu, Apr 12, 2018 at 12:33:50PM +0530, K K Subbu wrote:

> Hi,
>
> I stumbled upon the following method in OSProcess-Base package:
>
> ExternalPipe#atEnd
>
>  ^writer closed and: [self peek == nil]
>
> Shouldn't the test be reversed? In a pipe, the reader's peek buffer
> could be holding an object while the writer has closed its end.

Hi Subbu,

This one is a a bit tricky. An ExternalPipe represents both ends of
an OS pipe, from the class comment:

  I represent a pipe provided by the underlying operating system, such
  as a Unix pipe. I have a reader stream and a writer stream which
  behave similarly to a read-only FileStream and a writeable FileStream.

The atEnd method is, in a sense, wrong conceptually because it is checking
"writer closed", but in the general case, an instance of OSPipe (ExternalPipe)
that that is being used for reading connot possibly know the state of the
writer end of that pipe, because the pipe writer is probably running in
some other OS process.

I think (to the best of my recollection) that I wrote the method that way
in support of the unit tests. OSPipeTestCase provides some coverage of
OS pipe behavior within a single Squeak image, and if you change the
implementation of ExternalPipe>>atEnd to a more logically correct
implementation like this this:

  atEnd
      "Answer whether the receiver can access any more objects."
      ^ reader peek == nil

then the the unit tests will lock up the image.

So in summary: ExternalPipe>>atEnd is written is a way that allows
simple unit tests to be run within a single Squeak image. Given that
the rest of the unit tests also run and provide good coverage of pipe
usage, I would say that this is bad in theory but harmless in practice.

Dave