Skip to content

Disable PCNTL in forked processes#1196

Merged
labbati merged 32 commits into
masterfrom
labbati/pcntl-no-hang
May 5, 2021
Merged

Disable PCNTL in forked processes#1196
labbati merged 32 commits into
masterfrom
labbati/pcntl-no-hang

Conversation

@labbati

@labbati labbati commented Apr 7, 2021

Copy link
Copy Markdown
Member

Description

Motivation: pcntl installed (even when CLI tracing is not enabled) causes a delay of 5 seconds when the script exits (#1158)

What does this PR do?: it temporarily disable tracing on the forked process. Once we will use internal spans as the underlying mechanism for userland spans, as well, then we can support pcntl_fork properly.

Readiness checklist

  • (only for Members) Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

@labbati labbati added the c-extension Apply this label to issues and prs related to the C-extension label Apr 7, 2021
@labbati labbati added this to the 0.57.0 milestone Apr 7, 2021
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from 42605c8 to f6846d1 Compare April 9, 2021 10:01
@labbati labbati modified the milestones: 0.57.0, 0.57.1 Apr 12, 2021
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from 5ca53cd to 4420408 Compare April 13, 2021 16:31
@labbati labbati mentioned this pull request Apr 14, 2021
3 tasks
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from 4420408 to f7730f0 Compare April 14, 2021 12:44
Comment thread docker-compose.yml
- PHP_IDE_CONFIG=serverName=docker
cap_add:
- SYS_PTRACE
# Privileged is requires to run some pcntl tests locally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it here. The hope is that after I pass over it for 100 times, I will remember to enable it when I need to run the whole pcntl php language tests suite :D (which runs in CI)

@labbati labbati modified the milestones: 0.57.1, 0.59.0 Apr 21, 2021
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from f7730f0 to fca5bf5 Compare April 21, 2021 13:14
@labbati
labbati changed the base branch from master to labbati/ci-buster April 21, 2021 13:16
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch 2 times, most recently from 9882815 to c138755 Compare April 22, 2021 14:24
@labbati labbati mentioned this pull request Apr 22, 2021
5 tasks
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch 2 times, most recently from 2984d00 to bd845ab Compare April 23, 2021 16:49
Base automatically changed from labbati/ci-buster to master April 23, 2021 20:09
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from bd845ab to a4e4b86 Compare April 23, 2021 20:30
@labbati
labbati force-pushed the labbati/pcntl-no-hang branch from a4e4b86 to 8aa302d Compare April 26, 2021 09:11
labbati added 17 commits April 27, 2021 17:57
@labbati
labbati requested a review from SammyK May 4, 2021 16:27

@SammyK SammyK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once the microtime issue is squared away, this should be good to go!

@labbati

labbati commented May 5, 2021

Copy link
Copy Markdown
Member Author

@SammyK good catch on the time units in test. Just fixes it, do you mind taking a final look?

@SammyK SammyK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Fantastic work @labbati! 💯

@labbati

labbati commented May 5, 2021

Copy link
Copy Markdown
Member Author

Thanks @SammyK

@labbati
labbati merged commit 2f33678 into master May 5, 2021
@labbati
labbati deleted the labbati/pcntl-no-hang branch May 5, 2021 14:07
bwoebi added a commit that referenced this pull request Sep 1, 2025
Also avoid retrying a trace flush, that's pretty much pointless.

Signed-off-by: Bob Weinand <[email protected]>
bwoebi added a commit that referenced this pull request Sep 2, 2025
Also avoid retrying a trace flush, that's pretty much pointless.

Signed-off-by: Bob Weinand <[email protected]>
bwoebi added a commit that referenced this pull request Sep 4, 2025
Also avoid retrying a trace flush, that's pretty much pointless.

Signed-off-by: Bob Weinand <[email protected]>
bwoebi added a commit that referenced this pull request Sep 22, 2025
Also avoid retrying a trace flush, that's pretty much pointless.

Signed-off-by: Bob Weinand <[email protected]>
bwoebi added a commit that referenced this pull request Sep 29, 2025
* Clean shutdown of sidecar on SIGTERM (#1196)

Also avoid retrying a trace flush, that's pretty much pointless.

Signed-off-by: Bob Weinand <[email protected]>

* Update libdatadog

Signed-off-by: Bob Weinand <[email protected]>

---------

Signed-off-by: Bob Weinand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c-extension Apply this label to issues and prs related to the C-extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants