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

fix: install signal handlers should occur after plugins started #434

Merged

Conversation

YOU54F
Copy link
Member

@YOU54F YOU54F commented Jul 3, 2024

After splitting out a large PR of changes to apply #432 - I noted that I had previously applied the installed signal handlers, after the ffi framework has started the plugins.

Due to this, we are still encountering the issues in the project. Here is one on the mock server side

2024-07-03T20:13:07.907144Z DEBUG ThreadId(01) pact_models::pact: Writing new pact file to "/home/runner/work/pact-go/pact-go/examples/protobuf-message/../pacts/protobufmessageconsumer-protobufmessageprovider.json"
signal 17 received but handler not on signal stack
mp.gsignal stack [0xc000008000 0xc000010000], mp.g0 stack [0x7ffc7df38600 0x7ffc7ef37610], sp=0xc0000db398
fatal error: non-Go code set up signal handler without SA_ONSTACK flag

I actually wonder if on the verifier side, we will catch it in the pactffi_verifier_execute function, or if we need to actually install the signal handlers after this is called, discovering any required plugins from the pact sources, starting them. Then install the signal handlers, all before the pactffi_verifier_execute function returns.

I am going to test out a hypothesis in the pact-plugin-driver, but for now this appears to be pretty resiliant with multiple runs.

I'll be adding an additional PR with

  • Dockerfile
  • MacOS

and following on from that Windows support, however that is blocked as I have an issue with plugins failed to shutdown cleanly

@coveralls
Copy link

Coverage Status

coverage: 29.134%. remained the same
when pulling abd1815 on YOU54F:fix/issue_269_sa_onstack_flag_part2
into c80e188 on pact-foundation:master.

@mefellows mefellows merged commit 58a472a into pact-foundation:master Jul 4, 2024
8 checks passed
@YOU54F YOU54F deleted the fix/issue_269_sa_onstack_flag_part2 branch July 4, 2024 17:31
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