Menu

#2491 (Windows) Plotting data from stdin does not work properly.

None
closed-accepted
nobody
Windows (113)
2023-01-31
2022-02-07
No

Environment

  • Version: gnuplot 5.4.3 (Windows binary), and 5.5.0 (git head)
  • OS: Windows 10 (21H2) 64-bit

Overview

Plotting data read from stdin does not work properly.

How to reproduce the issue

1) Save the following text into file 'test.dem'.

plot '-' using 1:2 with linesp
0 0
1 1
2 4
3 9
4 16
e

pause 10

2) Execute gnuplot < test.dem
3) The plot point x=0, y=0 is missing from the plot window.

The cause

Reading data from stdin conflicts with the thread stdin_pipe_reader in winmain.c.

The patch to fix this issue

Revised (v3)

diff --git a/src/win/winmain.c b/src/win/winmain.c
index 85c12c0c3..acce228c5 100644
--- a/src/win/winmain.c
+++ b/src/win/winmain.c
@@ -1105,8 +1105,9 @@ stdin_pipe_reader(LPVOID param)
 {
     do {
    unsigned char c;
-   size_t n = fread(&c, 1, 1, stdin);
+   size_t n;
    WaitForSingleObject(input_cont, INFINITE);
+   n = fread(&c, 1, 1, stdin);
    if (n == 1)
        nextchar = c;
    else if (feof(stdin))
@@ -1122,9 +1123,9 @@ HANDLE
 init_pipe_input(void)
 {
     if (input_event == NULL)
-   input_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+   input_event = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (input_cont == NULL)
-   input_cont = CreateEvent(NULL, FALSE, TRUE, NULL);
+   input_cont = CreateEvent(NULL, FALSE, FALSE, NULL);
     if (input_thread == NULL)
    input_thread = CreateThread(NULL, 0, stdin_pipe_reader, NULL, 0, NULL);
     return input_event;
@@ -1135,7 +1136,7 @@ int
 next_pipe_input(void)
 {
     int c = nextchar;
-    SetEvent(input_cont);
+    ResetEvent(input_event);
     return c;
 }

@@ -1149,6 +1150,7 @@ ConsoleGetch(void)

     if (!_isatty(fd)) {
    h = init_pipe_input();
+   SetEvent(input_cont);
     } else {
    h = (HANDLE)_get_osfhandle(fd);
     }
1 Attachments

Related

Bugs: #2492
Bugs: #2511

Discussion

  • Takashi Yano

    Takashi Yano - 2022-02-07

    Sorry, the patch I posted first does not seem to work properly. The patch updated.

     

    Last edit: Takashi Yano 2022-02-07
  • Takashi Yano

    Takashi Yano - 2022-02-07

    Sorry, the updated patch still has problem. Revised again.

     

    Last edit: Takashi Yano 2022-02-07
  • Takashi Yano

    Takashi Yano - 2022-02-07
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -28,10 +28,11 @@
     Reading data from stdin conflicts with the thread `stdin_pipe_reader` in winmain.c.
    
    
    -# The patch to fix this issue
    +# The patch to fix this issue 
    +## Revised
     ~~~
     diff --git a/src/win/winmain.c b/src/win/winmain.c
    -index 85c12c0c3..f24d7914f 100644
    +index 85c12c0c3..acce228c5 100644
     --- a/src/win/winmain.c
     +++ b/src/win/winmain.c
     @@ -1105,8 +1105,9 @@ stdin_pipe_reader(LPVOID param)
    @@ -45,46 +46,33 @@
        if (n == 1)
            nextchar = c;
        else if (feof(stdin))
    -@@ -1124,22 +1125,12 @@ init_pipe_input(void)
    +@@ -1122,9 +1123,9 @@ HANDLE
    + init_pipe_input(void)
    + {
          if (input_event == NULL)
    -   input_event = CreateEvent(NULL, FALSE, FALSE, NULL);
    +-  input_event = CreateEvent(NULL, FALSE, FALSE, NULL);
    ++  input_event = CreateEvent(NULL, TRUE, FALSE, NULL);
          if (input_cont == NULL)
     -  input_cont = CreateEvent(NULL, FALSE, TRUE, NULL);
     +  input_cont = CreateEvent(NULL, FALSE, FALSE, NULL);
          if (input_thread == NULL)
        input_thread = CreateThread(NULL, 0, stdin_pipe_reader, NULL, 0, NULL);
          return input_event;
    +@@ -1135,7 +1136,7 @@ int
    + next_pipe_input(void)
    + {
    +     int c = nextchar;
    +-    SetEvent(input_cont);
    ++    ResetEvent(input_event);
    +     return c;
      }
    
    --
    --int
    --next_pipe_input(void)
    --{
    --    int c = nextchar;
    --    SetEvent(input_cont);
    --    return c;
    --}
    --
    --
    - int
    - ConsoleGetch(void)
    - {
    -@@ -1154,6 +1145,8 @@ ConsoleGetch(void)
    +@@ -1149,6 +1150,7 @@ ConsoleGetch(void)
    + 
    +     if (!_isatty(fd)) {
    +   h = init_pipe_input();
    ++  SetEvent(input_cont);
    +     } else {
    +   h = (HANDLE)_get_osfhandle(fd);
          }
    - 
    -     do {
    -+  if (!_isatty(fd))
    -+      SetEvent(input_cont);
    -   waitResult = MsgWaitForMultipleObjects(1, &amp;h, FALSE, INFINITE, QS_ALLINPUT);
    -   if (waitResult == WAIT_OBJECT_0) {
    -       if (_isatty(fd)) {
    -@@ -1161,7 +1154,7 @@ ConsoleGetch(void)
    -       if (c != NUL)
    -           return c;
    -       } else {
    --      return next_pipe_input();
    -+      return nextchar;
    -       }
    -   } else if (waitResult == WAIT_OBJECT_0+1) {
    -       WinMessageLoop();
     ~~~
    
    • Group: -->
    • Priority: -->
     
  • Takashi Yano

    Takashi Yano - 2022-02-07
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -29,7 +29,7 @@
    
    
     # The patch to fix this issue 
    -## Revised
    +## Revised (v3)
     ~~~
     diff --git a/src/win/winmain.c b/src/win/winmain.c
     index 85c12c0c3..acce228c5 100644
    
     
  • Tatsuro MATSUOKA

    I cannot reproduce on both 5.4.3 and 5.5.0 on windows.

     

    Last edit: Tatsuro MATSUOKA 2022-02-08
  • Takashi Yano

    Takashi Yano - 2022-02-08

    That's weird. I downloaded windows binary gp543-win64-mingw.7z from:
    https://sourceforge.net/projects/gnuplot/files/gnuplot/5.4.3/

    I can reproduce the problem in my several environments constantly. What is the difference?
    I started gnuplot.exe inside command prompt (cmd.exe).

    Does type test.dem | gnuplot result the same?

     
  • Tatsuro MATSUOKA

    For 5.4.3, I downloaded the gp543-win64-mingw.exe but execurable files should be the same as those in the 7zip archive. The gnuplot 5.5.0-mingw is built by myself for from the git-master.
    For OS, windows 10 (21H2).

    As you pointed out

    type test.dem | gnuplot 
    

    gives the same plot on gnuplot-5.4.3 and 5.5.0.

     

    Last edit: Tatsuro MATSUOKA 2022-02-08
  • Takashi Yano

    Takashi Yano - 2022-02-08

    Hmm, can anyone else reproduce the problem?

     
  • Bastian Märkisch

    When piping the script, I cannot reproduce the missing point at x=0. But I get an additional error :

    gnuplot> 0
             ^
             line 6: invalid command
    

    So there definitively is a bug. I'll investigate but it will take a few days. I note that running the script gnuplot test.dem or simply loading it works fine.

    Please note that input on Windows is a bit convoluted due to the many different possibilities (GUI/console/different internal and external "mousable" terminals). In particular there's additional code in qt.trm to support the qt terminal.

    This also means that it is important to know which terminal you are using. What is your GNUTERM environment variable set to?

     
    • Takashi Yano

      Takashi Yano - 2022-02-08

      Thanks for testing. "line 6: invalid command" error is also shown in my environment. Sorry for forgetting to mention that.

      I don't set GNUTERM environment, so qt terminal is used by default. However, the same happens with 'windows' or 'wxt' terminal.

       
  • Takashi Yano

    Takashi Yano - 2022-02-08

    This issue seems to depend on numbers of CPU cores.

    I tested again in my several PCs.

    Machine 1:
    CPU: AMD Ryzen 9 3950X (16 cores, 32 threads)
    Results: The issue always occurs.

    Machine 2:
    CPU: Intel Core i7-6700K (4 cores, 8 threads)
    Results: The issue always occurs.

    Machine 3:
    CPU: Intel Core i7-4790 (4 cores, 8 threads)
    Results: The issue always occurs.

    Machine 4:
    CPU: Intel Core i7-870 (4 cores, 8 threads)
    Results: The issue always occurs.

    Machine 5:
    CPU: Intel Core i5-M540 (2 cores, 4 threads)
    Results: The issue sometimes occurs.

    Machine 6:
    CPU: Intel Pentium D 930 (2 cores, 2 threads)
    Results: The issue never occurs.

    The problem is due to a race issue between some threads, so it is reasonable that it depends on the number of CPU cores.

     
  • Takashi Yano

    Takashi Yano - 2022-02-08

    This time I tried to verify by changing the number of CPU allocations in the virtual machine.

    CPU: Core i7-870 (2.93GHz)
    OS: Windows 10 1607 (Very old!)
    VM: VirtualBox 6.1.32

    Number of allocated CPUs: Result (Probability)
    1: Never occurs. (0%)
    2: Sometimes occurs. (about 50%)
    4: Always occurs. (100%)

     
  • Bastian Märkisch

    Thanks. I still do not fully understand what exactly is the cause of the race condition though (pointers greatly appreciated). Another problem is that the fgets_ipc(), which is used here, does not expect '\r' as line terminator. I am not yet completely sure where to best test for it. Currently, I add a test to next_pipe_input(), but that should probably be at a higher level.

    Your patch completely serializes the two threads. Doing the fread() before waiting for the event was supposed to somewhat speed up pipe input. Could you please try the attached variant of your fix? Unfortunately, my machine does not seem to have enough cores . ;) (Also I am confused that the number of cores >2 plays a role since we only employ two threads here.)

     
    • Takashi Yano

      Takashi Yano - 2022-02-15

      Thanks for the patch. Unfortunately, the patch pipe-fix-v4.diff does not solve the issue.

      The problem is that fgets() in MyFGetS() (or fread() in MyFRead() for binary data) and fread() in stdin_pipe_reader() thread try to read the stdin pipe simultaneously after reading the lineplot '-' using 1:2 with linesp. I mean this issue by the race issue.

      Serializing threads was to prevent this race issue. However, I came up with the new patch attached. The idea is: only the stdin_pipe_reader() reads stdin pipe directly, and MyFGetS() and MyFRead() retrieves the data from the buffer which stdin_pipe_reader() fills. With this patch, serialization is not necessary.

      What do you think?

       

      Last edit: Takashi Yano 2022-02-15
      • Takashi Yano

        Takashi Yano - 2022-02-16

        The patch is revised a bit.

         
        • Bastian Märkisch

          Thanks, I too had spotted the additional case in MyFGetC().

           
      • Bastian Märkisch

        Thanks for pointing out the issue! I had initially assumed I screwed up the thread sync.
        Limiting pipe input to one single mechanism seems like a great idea iny any case.

        To be conservative, I consider (a variant of) your initial patch for 5.4 and this one for 5.5 aka master branch. I very much like your solution, but it should get more testing. We just had too many issues with IO in 5.4 for console mode. What do you think?

        Btw. have you verified with the qt terminal? qt_waitforinput() is an even more complex variant of the message loop in winmain.c because it needs to take care of the communication with the external qt terminal. Ideally, that should be really moved to winmain.c at some stage.

         
        • Takashi Yano

          Takashi Yano - 2022-02-16

          I had tested only with windows terminal. So, I have made tests for my initial patch and the patch tyan0-20220216.patch with qt and wxt terminal as well just now.

          • Initial patch: qt terminal does not work at all.
          • Revised initial patch (attached): qt terminal works with text interface, however, if using with GNU Octave (which passes binary data via stdin), the second plotting hangs. After several minutes hang, plotting window is occasionally updated to new plot. (windows and wxt terminal work fine both for text interface and GNU Octave.)
          • tyan0-20220216.patch: qt, wxt and windows terminal work as expected even with GNU Octave.

          The hang of qt terminal in GNU Octave is also happen with gnuplot 5.2.8. So, This may be another problem. Anyway, the patch tyan0-20220216.patch solves both problems.

          So I think applying the second patch (tyan0-20220216.patch) for both 5.4 and 5.5 is better.

           
          • Takashi Yano

            Takashi Yano - 2022-02-16

            The hang of qt terminal in GNU Octave is also happen with gnuplot 5.2.8. So, This may be another problem.

            Ah, hang in 5.2.8 is not cleared even after several minutes, while the hang with revised initial patch for git master is cleared in several minutes. So, these may be not the same problem.

             
  • Ethan Merritt

    Ethan Merritt - 2022-06-04
    • summary: Plotting data from stdin does not work properly. --> (Windows) Plotting data from stdin does not work properly.
     
  • Bastian Märkisch

    • labels: --> Windows
    • status: open --> pending-accepted
     
  • Bastian Märkisch

    Finally in 5.4 and 5.5 branches. Thanks!

     
  • Takashi Yano

    Takashi Yano - 2022-10-30

    I confirmed that the issue was fixed in the head of master (5.5) and branch-5-4-stable. Thanks!

     
  • Ethan Merritt

    Ethan Merritt - 2023-01-31
    • status: pending-accepted --> closed-accepted
     

Log in to post a comment.