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

Jump to bottom

Consolidate the different versions of TEMP_FAILURE_RETRY_BLOCK_SIGNALS #16927

Closed
iposva-google opened this issue Feb 19, 2014 · 5 comments
Closed

Consolidate the different versions of TEMP_FAILURE_RETRY_BLOCK_SIGNALS #16927

iposva-google opened this issue Feb 19, 2014 · 5 comments
Assignees
@johnmccutchan
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@iposva-google
Copy link
Contributor

We have both TEMP_FAILURE_RETRY_BLOCK_SIGNALS and TEMP_FAILURE_RETRY in our sources. In the presence of the profiler we should not ever use TEMP_FAILURE_RETRY as it has the potential of spinning without making any forward progress.
@andersjohnsen
Copy link

Can you expand on the spinning issue when the profiler is on?

@johnmccutchan
Copy link
Contributor

In the worst case what can happen when using TEMP_FAILURE_RETRY to wrap system calls is:

10 thread enters system call
20 thread receives SIGPROF signal
30 thread exits system call with EINTR
40 GOTO 10

By using TEMP_FAILURE_RETRY_BLOCK_SIGNALS the program will never get stuck in that loop. Given that we expect the profiler to always be enabled (today it is enabled on all Linux and Mac bots) it doesn't make sense to ever use TEMP_FAILURE_RETRY instead of TEMP_FAILURE_RETRY_BLOCK_SIGNALS. We should remove TEMP_FAILURE_RETRY.

@andersjohnsen
Copy link

I agrre, but what about replacing TEMP_FAILURE_RETRY with the logic of
TEMP_FAILURE_RETRY_BLOCK_SIGNALS?

TEMP_FAILURE_RETRY_BLOCK_SIGNALS is a very long macro name :)

@johnmccutchan
Copy link
Contributor

sgtm

@iposva-google iposva-google added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Feb 19, 2014
@johnmccutchan
Copy link
Contributor

I believe this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@johnmccutchan johnmccutchan

Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kevmoo @andersjohnsen @johnmccutchan @iposva-google