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

core: Detect initial timer state from serialized data #10778

Merged
merged 1 commit into from Nov 21, 2018

Conversation

Werkov
Copy link
Contributor

@Werkov Werkov commented Nov 14, 2018

We keep a mark whether a single-shot timer was triggered in the caller's
variable initial. When such a timer elapses while we are
serializing/deserializing the inner state, we consider the timer
incorrectly as elapsed and don't trigger it later.

This patch exploits last_trigger timestamp that we already serialize,
hence we can eliminate the argument initial completely.

A reproducer for OnBootSec= timers:

    cat >repro.c <<EOD
    /*
     * Compile:	gcc repro.c -o repro
     * Run:		./repro
     */
    #include <errno.h>
    #include <fcntl.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <sys/stat.h>
    #include <sys/types.h>
    #include <time.h>
    #include <unistd.h>

    int main(int argc, char *argv[]) {
    	char command[1024];
    	int pause;

    	struct timespec now;

    	while (1) {
    		usleep(rand() % 200000); // prevent periodic repeats
           		clock_gettime(CLOCK_MONOTONIC, &now);
    		printf("%i\n", now.tv_sec);

    		system("rm -f $PWD/mark");
    		snprintf(command, 1024, "systemd-run --user --on-boot=%i --timer-property=AccuracySec=100ms "
    					"touch $PWD/mark", now.tv_sec + 1);
    		system(command);
    		system("systemctl --user list-timers");
    		pause = (1000000000 - now.tv_nsec)/1000 - 70000; // fiddle to hit the middle of reloading
    		usleep(pause > 0 ? pause : 0);
    		system("systemctl --user daemon-reload");
    		sync();
    		sleep(2);
    		if (open("./mark", 0) < 0)
    			if (errno == ENOENT) {
    				printf("mark file does not exist\n");
    				break;
    			}
    	}

    	return 0;
    }
    EOD

We keep a mark whether a single-shot timer was triggered in the caller's
variable initial. When such a timer elapses while we are
serializing/deserializing the inner state, we consider the timer
incorrectly as elapsed and don't trigger it later.

This patch exploits last_trigger timestamp that we already serialize,
hence we can eliminate the argument initial completely.

A reproducer for OnBootSec= timers:
        cat >repro.c <<EOD
        /*
         * Compile:	gcc repro.c -o repro
         * Run:		./repro
         */
        #include <errno.h>
        #include <fcntl.h>
        #include <stdio.h>
        #include <stdlib.h>
        #include <sys/stat.h>
        #include <sys/types.h>
        #include <time.h>
        #include <unistd.h>

        int main(int argc, char *argv[]) {
        	char command[1024];
        	int pause;

        	struct timespec now;

        	while (1) {
        		usleep(rand() % 200000); // prevent periodic repeats
               		clock_gettime(CLOCK_MONOTONIC, &now);
        		printf("%i\n", now.tv_sec);

        		system("rm -f $PWD/mark");
        		snprintf(command, 1024, "systemd-run --user --on-boot=%i --timer-property=AccuracySec=100ms "
        					"touch $PWD/mark", now.tv_sec + 1);
        		system(command);
        		system("systemctl --user list-timers");
        		pause = (1000000000 - now.tv_nsec)/1000 - 70000; // fiddle to hit the middle of reloading
        		usleep(pause > 0 ? pause : 0);
        		system("systemctl --user daemon-reload");
        		sync();
        		sleep(2);
        		if (open("./mark", 0) < 0)
        			if (errno == ENOENT) {
        				printf("mark file does not exist\n");
        				break;
        			}
        	}

        	return 0;
        }
        EOD
@yuwata yuwata added the pid1 label Nov 15, 2018
@poettering
Copy link
Member

Neat, I like that this is a simplification.

@poettering
Copy link
Member

CI failures appear unrelated.

@poettering poettering merged commit aa1f95d into systemd:master Nov 21, 2018
@nh2 nh2 mentioned this pull request Apr 30, 2021
facebook-github-bot pushed a commit to facebook/chef-cookbooks that referenced this pull request Jun 15, 2022
Summary:

Adding logic to phase out the fb_timers 'Requires=' directive that was added to address an outstanding bug which was fixed here: systemd/systemd#10778

The Requires= directive is causing services to fire immediately at boot time, thereby ignoring the whole logic/point of using the Persistent=false & OnCalendar= directives.

Differential Revision: D37013147

fbshipit-source-id: 66d3cfca12320e0310b1eb8eb9755c0d509742bd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants