Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PipeFail test does not finish #25

Closed
mmrazik opened this issue Mar 27, 2021 · 4 comments
Closed

PipeFail test does not finish #25

mmrazik opened this issue Mar 27, 2021 · 4 comments

Comments

@mmrazik
Copy link

mmrazik commented Mar 27, 2021

While trying to root-cause #2 I ran into a different issue.

I am running with a modified PipeFail.ino. With 2000 I can't reproduce this issue nor #2.

diff --git a/examples/PipeFail/PipeFail.ino b/examples/PipeFail/PipeFail.ino
index c46087d..d2e7ee6 100644
--- a/examples/PipeFail/PipeFail.ino
+++ b/examples/PipeFail/PipeFail.ino
@@ -18,7 +18,7 @@
 // * Ubuntu 20.04: 300-1000
 // * MacOS 10.13: ~1000
 // * FreeBSD 12: 1000-2000
-const int NUM_LINES = 2000;
+const int NUM_LINES = 20000;
 const char LINE[] = "Reproduce https://github.com/bxparks/EpoxyDuino/issues/2";
 
 void setup() {

Now when I run PipeFail.out it looks like this:

mmrazik@t7610:~/github/EpoxyDuino/examples$ PipeFail/PipeFail.out
0 (0): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
1 (67): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
2 (134): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
3 (201): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
...
14784 (990528): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
14785 (990595): Reproduce https://github.com/bxparks/EpoxyDuino/issues/2
mmrazik@t7610:~/github/EpoxyDuino/examples$ /bxparks/EpoxyDuino/issues/2

It fails at different places but usually after 10k iterations.

It turns out the putchar in StdioSerial::write returns EOF. When I played with it turned out errno is set to EAGAIN (Resource temporarily unavailable).

The following patch fixes it for me:

index b1b985c..9004e22 100644
--- a/StdioSerial.cpp
+++ b/StdioSerial.cpp
@@ -4,10 +4,18 @@
  */
 
 #include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
 #include "StdioSerial.h"
 
 size_t StdioSerial::write(uint8_t c) {
   int result = putchar(c);
+  int max_retries = 10;
+  while (result == EOF && errno == EAGAIN && max_retries > 0) {
+    max_retries--;
+    usleep(1000);
+    result = putchar(c);
+  }
   return (result == EOF) ? 0 : 1;
 }
@bxparks
Copy link
Owner

bxparks commented Apr 5, 2021

Thanks for spending some time to look into this issue. It's weird to me that the problem is triggered with different number of lines or characters. Even on the same machine, I think I get slightly random failure points. As I noted in #2, I suspect the primary problem is that both the main.cpp of EpoxyDuino and the less program want to grab the keyboard input in "raw" mode, and they interfere with each other. The workaround that I noted in #2 (redirecting the stdin from /dev/null) always works for me. And redirecting the output to a pipe (e.g. grep) or a file (> file.out) always works, so I have not had a huge urgency to fix this.

@supercc-arduino
Copy link

supercc-arduino commented Aug 27, 2021

Hello,

The ICANON mode, fixed for stdin, seems to cause the brutal termination of Putchar when the buffer is full. To avoid this simply replace putchar by the POSIX write system call (2).

New version of cores/StdioSerial.cpp that works for me:

/*
 * Copyright (c) 2019 Brian T. Park
 * MIT License
 */

#include <stdio.h>
#include <unistd.h>

#include "StdioSerial.h"

size_t StdioSerial::write(uint8_t c) {

  int result=::write(STDOUT_FILENO, &c, 1);

  return (result == EOF) ? 0 : 1;
}

void StdioSerial::flush() {
    fflush(stdout);
}

StdioSerial Serial;

Thanks for Epoxy ;-)

@bxparks
Copy link
Owner

bxparks commented Sep 30, 2021

@supercc-arduino: Thanks for the note. I'm not sure that::write() fixes the problem. If I understand things correctly, it bypasses the <stdio.h> buffer (which makes the fflush(stdout) a no-op). But it does not seem to actually solve the problem of both PipeFail.out and less fighting for the stdin. Because after making your change, $ ./PipeFail.out | less still fails with an exit code of 1, but ./PipeFail.out < /dev/null | less succeeds.

bxparks added a commit that referenced this issue Sep 30, 2021
@bxparks
Copy link
Owner

bxparks commented Sep 30, 2021

I believe I have finally fixed this with bf6248f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants