bugGNU Octave - Bugs: bug #47614, pipe() does not work on Windows

 
 

bug #47614: pipe() does not work on Windows

Submitted by:  Tatsuro MATSUOKA <tmacchant>
Submitted on:  Mon 04 Apr 2016 03:03:34 AM UTC  
 
Category: Octave FunctionSeverity: 1 - Wish
Priority: 2Item Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Tatsuro MATSUOKAOpen/Closed: Closed
Release: devOperating System: Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 07 Apr 2016 08:30:41 PM UTC, comment #18:

I see the same on Debian, fgets only reads to the first newline by definition, they are probably still buffered, not related to this bug.

Mike Miller <mtmiller>
Project Administrator
Thu 07 Apr 2016 08:05:59 PM UTC, comment #17:

Just to confirm, this now works fine on 4.1.0+

The example in the original post works but 3 of the 4 newlines got eaten.
Do we need to care about that?

Philip Nienhuis <philipnienhuis>
Project Member
Wed 06 Apr 2016 11:53:34 PM UTC, comment #16:

Moving back to the maintainers list to answer this.

Mike Miller <mtmiller>
Project Administrator
Wed 06 Apr 2016 11:13:09 PM UTC, comment #15:

I would like test John's patch. However, I lacks knowledge how do I treat it.

My questions are:

1. bootstrap.conf : How do I reflect this in mxe build.

2. oct-syscalls.cc : Prepare stable-octave-1-xxxxx.patch (xxxx is a
temporal name here) that include corresponding patch file #36846
to oct-syscalls.cc.

Tatsuro MATSUOKA <tmacchant>
Wed 06 Apr 2016 07:59:01 PM UTC, comment #14:

Thanks for testing, and thanks everyone for the help on this. I've pushed this to the stable branch and merged it to default, so pipe should now work on Windows builds going forward (not 4.0.1, but someone could patch it and rebuild or make a build from the stable branch with this fix).

Mike Miller <mtmiller>
Project Administrator
Wed 06 Apr 2016 04:02:23 PM UTC, comment #13:

On stable, with the changes from this attached diff, it works for me.

(file #36846)

John Donoghue <lostbard>
Project Member
Wed 06 Apr 2016 10:22:49 AM UTC, comment #12:

I will try it later today unless someone else gets to it first

John Donoghue <lostbard>
Project Member
Wed 06 Apr 2016 04:36:42 AM UTC, comment #11:

Ok, good, we're closing in on this one. So if we enable pipe-posix, we should simply remove the HAVE_PIPE conditional in oct-syscalls.cc.

The gnulib logic should enable the pipe.c module if and only if HAVE_PIPE is false, so with it enabled there should always be a pipe function that can be linked to.

Can someone test another build with that change made? I can provide a patch if it's not clear what should be done.

Mike Miller <mtmiller>
Project Administrator
Wed 06 Apr 2016 02:33:23 AM UTC, comment #10:

looks like pipe.c is compiled, however HAVE_PIPE is commented out in config.h

John Donoghue <lostbard>
Project Member
Tue 05 Apr 2016 11:01:43 PM UTC, comment #9:

I'll try on mine tommorrow unless someone else beats me to it

John Donoghue <lostbard>
Project Member
Tue 05 Apr 2016 06:42:45 PM UTC, comment #8:

Hmm, it should have, but maybe syscalls needs some tweaking also. Thanks for double checking.

Enabling the "pipe-posix" module should do everything needed to make the pipe function available, but maybe HAVE_PIPE is still unset.

Can someone with the pipe-posix module enabled check whether HAVE_PIPE is set or unset in config.h? And whether the libgnu/pipe.c module is compiled during the build or not?

Mike Miller <mtmiller>
Project Administrator
Tue 05 Apr 2016 05:40:23 PM UTC, comment #7:

On my system -compiling octave 4.0.1, with a patch similar to Mikes, but directly added to octave_syscalls::pipe , it works for me.

I did notice that if not enough newlines are added, it gets stuck in the fgets and the olnly way to exit is ctrl-c to terminate octave.

fgets trying to buffer the reads?

It did use the gnulib version with the patch?
http://octave.1599824.n4.nabble.com/attachment/4676026/0/oct-gnulib-pipe.diff

(file #36842)

John Donoghue <lostbard>
Project Member
Tue 05 Apr 2016 07:03:28 AM UTC, comment #6:

The snippet of the log file you posted is actually a little misleading. Octave is not actually using the gnulib pipe module normally, except with the patch I provided Philip for testing.

But you are right that if we did use the gnulib pipe replacement, it's a wrapper around _pipe, and that does not work for Octave as Philip showed in his testing.

Mike Miller <mtmiller>
Project Administrator
Mon 04 Apr 2016 10:29:01 PM UTC, comment #5:

I have yet built development branch of octave with mxe-octave.
(I am now building.)

Seeing log file of stable-octave,

Octave uses the gnulib pipe.
The gnulib pipe does nothing on the posix system and replace pipe by
_pipe on Windows.

> Another option is to keep things the way they are now. The pipe function currently throws an error:


At the moment, I also think that it is better to do so.

Tatsuro MATSUOKA <tmacchant>
Mon 04 Apr 2016 04:38:55 PM UTC, comment #4:

Another option is to keep things the way they are now. The pipe function currently throws an error:

Windows users could use other IPC mechanisms such as sockets.

Mike Miller <mtmiller>
Project Administrator
Mon 04 Apr 2016 03:47:07 AM UTC, comment #3:

On octave on native windows

On the other hand, octave on Cygwin:

The "ans" values are changed by repetition of "pipe()".

In the comment, the gnulib implementation of pipe
http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/pipe.c
at line 34-35:

From the above, the pipe seems to be implemented using _pipe on
windows but that seems not to work as is pointed out by Mike in the ML.

We have to consider implement pipe fucntion using Windows API
as also indicated by Mike in the ML.

Tatsuro MATSUOKA <tmacchant>
Mon 04 Apr 2016 03:31:42 AM UTC, comment #2:

A little struggle:

is defined in oct-syscalls.cc
http://octave.org/doxygen/4.0/d5/d5e/oct-syscalls_8cc_source.html

Tatsuro MATSUOKA <tmacchant>
Mon 04 Apr 2016 03:22:03 AM UTC, comment #1:

I have a quick looked at into libinterp/corefcn/syscalls.cc

http://octave.org/doxygen/4.1/d7/dba/syscalls_8cc_source.html

in the above, I could see
The above things are what I have done at this moment.

Tatsuro MATSUOKA <tmacchant>
Mon 04 Apr 2016 03:03:34 AM UTC, original submission:

The topic has been discussed in Octave-Maintainers Mailing list:

http://octave.1599824.n4.nabble.com/optim-struct-parallel-pre-release-check-on-Windows-td4675995.html

Octave for windows seems to implement pipe();

However, that does not work correctly using the test by Mike:
http://octave.1599824.n4.nabble.com/optim-struct-parallel-pre-release-check-on-Windows-td4675995.html#a4676026

On octave on CygwinX64, the above work as expected.

However, octave for (Native) windows, the above give incorrect results:

Implementation pipe for native window is not correct.

At this time, I have not look into libinterp/corefcn/syscalls.cc
but this is the start, I think

Tatsuro MATSUOKA <tmacchant>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #36846:  pipe.diff added by lostbard (658B - application/octet-stream)
file #36842:  stable-octave-2-pipe-fixes.patch added by lostbard (701B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by lostbard (Updated the item)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by tmacchant (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 06 Apr 2016 07:59:01 PM UTCmtmillerStatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Wed 06 Apr 2016 04:02:23 PM UTClostbardAttached File-=>Added pipe.diff, #36846
    Tue 05 Apr 2016 05:40:23 PM UTClostbardAttached File-=>Added stable-octave-2-pipe-fixes.patch, #36842
    Mon 04 Apr 2016 04:38:55 PM UTCmtmillerSeverity3 - Normal=>1 - Wish
      Priority5 - Normal=>2
      StatusNone=>Confirmed
      Release4.0.1=>dev

    Back to the top


    Powered by Savane 3.1-cleanup1