Skip to content

Fix itimer state restoration in filelock#53

Draft
Copilot wants to merge 1 commit intomainfrom
copilot/fix-42
Draft

Fix itimer state restoration in filelock#53
Copilot wants to merge 1 commit intomainfrom
copilot/fix-42

Conversation

Copy link

Copilot AI commented Sep 19, 2025

Currently filelock restores the SIGALRM handler to its previous state but the itimer state is not restored. This can disrupt existing timers that were running before the filelock operation, such as R's profiling timer used by Rprof().

The issue occurs because while filelock correctly saves and restores the signal handler using sigaction(), it doesn't save and restore the timer state when calling setitimer(). This means any existing timer gets lost when filelock sets its own timer for timeout handling.

Changes Made

This PR adds minimal changes to properly save and restore the itimer state:

  1. Added global variable filelock_old_timer to store the previous timer state
  2. Modified setitimer call to capture existing timer state before setting new one
  3. Updated alarm callback to restore both signal handler and timer state
// Before: timer state was lost
setitimer(ITIMER_REAL, &timer, 0);

// After: timer state is preserved  
setitimer(ITIMER_REAL, &timer, &filelock_old_timer);

The alarm callback now restores both the signal handler and the timer:

void filelock__alarm_callback (int signum) {
  /* Restore signal handler */
  sigaction(SIGALRM, &filelock_old_sa, 0);
  /* Restore timer state */
  setitimer(ITIMER_REAL, &filelock_old_timer, 0);
}

Impact

This fix allows systems like R's profiling (Rprof()) to resume normal operation after filelock completes, as the profiling timer will be properly restored along with its interval settings. The handler will have missed some intervals during the filelock operation, but this is expected behavior since timer intervals are only a lower bound and handlers should account for this possibility.

Fixes #42.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • cloud.r-project.org
    • Triggering command: /usr/lib/R/bin/exec/R -e install.packages(c('testthat',~+~'callr'),~+~repos='REDACTED') (dns block)
  • cran.rstudio.com
    • Triggering command: /usr/lib/R/bin/exec/R -e options(repos~+~=~+~'REDACTED');~+~install.packages(c('testthat',~+~'callr')) (dns block)
  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Author

Copilot AI commented Sep 19, 2025

@krlmlr 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI changed the title [WIP] itimer state is not restored Fix itimer state restoration in filelock Sep 19, 2025
Copilot AI requested a review from krlmlr September 19, 2025 17:32
- Add global variable filelock_old_timer to store previous timer state
- Modify setitimer call to save old timer state before setting new one
- Modify alarm callback to restore both signal handler and timer state
- Verified package builds and installs correctly
- Tested functionality with multiple scenarios

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

I reviewed the code and purged useless changes but didn't change otherwise.

@krlmlr krlmlr requested a review from gaborcsardi September 19, 2025 18:35
@krlmlr
Copy link
Member

krlmlr commented Sep 19, 2025

@gaborcsardi: @lionel- asked me to ping you for a review. The code looks legit, but I wouldn't know how to test it.

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.

itimer state is not restored

2 participants