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

Profile: Minor fixes. Signal handling fix. #44199

Merged
merged 6 commits into from
Feb 17, 2022

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth commented Feb 16, 2022

Addressing some post-merge review suggestions from #44185 (review)

@IanButterworth
Copy link
Sponsor Member Author

IanButterworth commented Feb 17, 2022

@vtjnash I've seen some rare instances of this kind of behavior on linux with regular profiling

julia> @profile begin
           sleep(1)
       end

======================================================================================
Information request received. A stacktrace will print followed by a 1.0 second profile
======================================================================================

Also there are some spurious info request prints in the linux CI, where the only ones Profile triggers are sent to an io buffer, so something is unintentionally triggering these.
https://buildkite.com/julialang/julia-master/builds/8929#ed106d69-576a-4584-bdb7-4c45a0288f83/282-813

Given SIGUSR1 is now multipurpose, I think it's that running = 0 is being declared too early and some straggling timer-emitted signal gets confused for a user-sent one.

I suspect the issue may be that the timer might still spit out a signal after timer_delete returns, which AFAIU is explained by https://man7.org/linux/man-pages/man2/timer_delete.2.html

The treatment of any pending signal generated by the deleted timer is unspecified.

So I added the 93dce80 to add some buffer to let a stray signal pass before declaring running = 0

There may be a better way to do this, or my understanding may be flawed.

@IanButterworth IanButterworth changed the title Profile: Minor fixes Profile: Minor fixes. Signal handling fix. Feb 17, 2022
@IanButterworth
Copy link
Sponsor Member Author

There may be a better fix than what is described in #44199 (comment)

But I think it would be good to get these in given there are unexplained intermittent failures on package_linux64 which didn't happen here.

@IanButterworth IanButterworth merged commit 072c041 into JuliaLang:master Feb 17, 2022
@IanButterworth IanButterworth deleted the ib/profile_fixes branch February 17, 2022 17:52
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
@IanButterworth
Copy link
Sponsor Member Author

Hasn't appeared to fix the spurious triggers on linux
https://buildkite.com/julialang/julia-master/builds/8977#55fe445b-9756-4357-9f75-5048cbd202e8/281-803

Perhaps the sleep is too short?

KristofferC pushed a commit that referenced this pull request Feb 18, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Feb 24, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
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