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

Report some read errors back to java code. #155

Merged

Conversation

hiddenalpha
Copy link

No description provided.

@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from 30ddacc to 03168ad Compare November 14, 2023 23:56
@hiddenalpha hiddenalpha marked this pull request as draft November 14, 2023 23:58
@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from 03168ad to f6976c1 Compare November 15, 2023 00:09
@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from f6976c1 to 772f9be Compare November 15, 2023 00:19
@hiddenalpha hiddenalpha marked this pull request as ready for review November 15, 2023 00:34
@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from a6b8de8 to e0f22a5 Compare November 18, 2023 16:52
@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from e0f22a5 to 661987e Compare November 19, 2023 13:38
@tresf
Copy link

tresf commented Nov 19, 2023

For some reason the Intel Mac job keeps getting hung in CI. I'm going to see if I can determine if it's related to this PR or not before merging.

@tresf
Copy link

tresf commented Nov 19, 2023

The hang definitely appears to occur during SerialNativeInterfaceTest.... What's weird is the CI just completely hangs and all of the mvn logs vanish once it gets to that point, as if we're somehow conflicting with the pipe... I'll see if I can reproduce locally.

@tresf
Copy link

tresf commented Nov 19, 2023

I can reproduce the hang locally. Investigating.

@tresf
Copy link

tresf commented Nov 19, 2023

@hiddenalpha although I can reproduce the deadlock, I'm not sure why it's happening...

  • On MacOS, when the test throwsIllegalArgumentExceptionIfPortHandleIllegal runs, it hangs indefinitely.
  • I tried to add some printf statements to the C++ code, but could not find a way to display them from Junit.
  • I tried to break when result == 0, but it doesn't affect the deadlock.
  • I tried to provide different portHandle values, but they either behave the same or fail the test.

We may decide to just skip this test for MacOS, but before we make that decision, I'd like to know why. I can offer a Mac environment via SSH if needed.

@pietrygamat
Copy link
Collaborator

The hanging test on MacOS seems related to poll vs select switch that causes MacOS use different path. From what I could tell, it is not related to the C code changes - the same test on master would get stuck the same way. Defning #define HAVE_POLL 1 for APPLE as well as for other OSes results in a test pass, but that gets us back this:

    // Seems as poll has some portability issues on OsX (Search for "poll" in
    // "https://cr.yp.to/docs/unixport.html"). So we only make use of poll on
    // all platforms except "__APPLE__".
    // If you want to force usage of 'poll', pass "-DHAVE_POLL=1" to gcc.

and perhaps this and maybe this as well #53.

Round 1: - Fix typo in exception msg.
         - Indent 'case' more.
         - use variable in place of constant.
Round 2: - Ignore test on windoof.
         - Whops, did ignore the wrong test (force-pushed to fix the
           other one).
Round 3: - Enable windoof test again as requested in PR review.
Round 4: - Move failing OSX to its own test.

Taken from 3dcb4ec.
@hiddenalpha hiddenalpha force-pushed the handle-some-read-errors-20231115 branch from 661987e to 98258e1 Compare November 21, 2023 12:01
@tresf tresf merged commit 68cce35 into java-native:master Nov 21, 2023
13 checks passed
@hiddenalpha hiddenalpha deleted the handle-some-read-errors-20231115 branch November 21, 2023 18:55
@pietrygamat pietrygamat added this to the 2.9.6 milestone Dec 1, 2023
@hiddenalpha hiddenalpha restored the handle-some-read-errors-20231115 branch February 16, 2024 20:49
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

Successfully merging this pull request may close these issues.

3 participants