author | Lars Hjemli <hjemli@gmail.com> | 2006-12-11 21:53:50 (UTC) |
---|---|---|
committer | Lars Hjemli <hjemli@gmail.com> | 2006-12-11 21:53:50 (UTC) |
commit | fbaf1171b4e343929dd43ecac7cd9d1c692b84ec (patch) (unidiff) | |
tree | 40b7007b2e1e85f2e91e052b33555df2e9fb5e14 | |
parent | 44923f8953c66dc9b852316b655ab3b5aec9478e (diff) | |
download | cgit-fbaf1171b4e343929dd43ecac7cd9d1c692b84ec.zip cgit-fbaf1171b4e343929dd43ecac7cd9d1c692b84ec.tar.gz cgit-fbaf1171b4e343929dd43ecac7cd9d1c692b84ec.tar.bz2 |
Don't truncate valid cachefiles
An embarrassing thinko in cgit_check_cache() would truncate valid cachefiles
in the following situation:
1) process A notices a missing/expired cachefile
2) process B gets scheduled, locks, fills and unlocks the cachefile
3) process A gets scheduled, locks the cachefile, notices that the cachefile
now exist/is not expired anymore, and continues to overwrite it with an
empty lockfile.
Thanks to Linus for noticing (again).
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
-rw-r--r-- | cache.c | 5 | ||||
-rw-r--r-- | cgit.c | 14 | ||||
-rw-r--r-- | cgit.h | 1 |
3 files changed, 16 insertions, 4 deletions
@@ -78,30 +78,35 @@ int cache_lock(struct cacheitem *item) | |||
78 | 78 | ||
79 | top: | 79 | top: |
80 | if (++i > cgit_max_lock_attempts) | 80 | if (++i > cgit_max_lock_attempts) |
81 | die("cache_lock: unable to lock %s: %s", | 81 | die("cache_lock: unable to lock %s: %s", |
82 | item->name, strerror(errno)); | 82 | item->name, strerror(errno)); |
83 | 83 | ||
84 | item->fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR); | 84 | item->fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR); |
85 | 85 | ||
86 | if (item->fd == NOLOCK && errno == ENOENT && cache_create_dirs()) | 86 | if (item->fd == NOLOCK && errno == ENOENT && cache_create_dirs()) |
87 | goto top; | 87 | goto top; |
88 | 88 | ||
89 | if (item->fd == NOLOCK && errno == EEXIST && | 89 | if (item->fd == NOLOCK && errno == EEXIST && |
90 | cache_refill_overdue(lockfile) && !unlink(lockfile)) | 90 | cache_refill_overdue(lockfile) && !unlink(lockfile)) |
91 | goto top; | 91 | goto top; |
92 | 92 | ||
93 | return (item->fd > 0); | 93 | return (item->fd > 0); |
94 | } | 94 | } |
95 | 95 | ||
96 | int cache_unlock(struct cacheitem *item) | 96 | int cache_unlock(struct cacheitem *item) |
97 | { | 97 | { |
98 | close(item->fd); | 98 | close(item->fd); |
99 | return (rename(fmt("%s.lock", item->name), item->name) == 0); | 99 | return (rename(fmt("%s.lock", item->name), item->name) == 0); |
100 | } | 100 | } |
101 | 101 | ||
102 | int cache_cancel_lock(struct cacheitem *item) | ||
103 | { | ||
104 | return (unlink(fmt("%s.lock", item->name)) == 0); | ||
105 | } | ||
106 | |||
102 | int cache_expired(struct cacheitem *item) | 107 | int cache_expired(struct cacheitem *item) |
103 | { | 108 | { |
104 | if (item->ttl < 0) | 109 | if (item->ttl < 0) |
105 | return 0; | 110 | return 0; |
106 | return item->st.st_mtime + item->ttl * 60 < time(NULL); | 111 | return item->st.st_mtime + item->ttl * 60 < time(NULL); |
107 | } | 112 | } |
@@ -40,55 +40,61 @@ static void cgit_fill_cache(struct cacheitem *item) | |||
40 | { | 40 | { |
41 | htmlfd = item->fd; | 41 | htmlfd = item->fd; |
42 | item->st.st_mtime = time(NULL); | 42 | item->st.st_mtime = time(NULL); |
43 | if (cgit_query_repo) | 43 | if (cgit_query_repo) |
44 | cgit_print_repo_page(item); | 44 | cgit_print_repo_page(item); |
45 | else | 45 | else |
46 | cgit_print_repolist(item); | 46 | cgit_print_repolist(item); |
47 | } | 47 | } |
48 | 48 | ||
49 | static void cgit_check_cache(struct cacheitem *item) | 49 | static void cgit_check_cache(struct cacheitem *item) |
50 | { | 50 | { |
51 | int i = 0; | 51 | int i = 0; |
52 | 52 | ||
53 | cache_prepare(item); | 53 | cache_prepare(item); |
54 | top: | 54 | top: |
55 | if (++i > cgit_max_lock_attempts) { | 55 | if (++i > cgit_max_lock_attempts) { |
56 | die("cgit_refresh_cache: unable to lock %s: %s", | 56 | die("cgit_refresh_cache: unable to lock %s: %s", |
57 | item->name, strerror(errno)); | 57 | item->name, strerror(errno)); |
58 | } | 58 | } |
59 | if (!cache_exist(item)) { | 59 | if (!cache_exist(item)) { |
60 | if (!cache_lock(item)) { | 60 | if (!cache_lock(item)) { |
61 | sleep(1); | 61 | sleep(1); |
62 | goto top; | 62 | goto top; |
63 | } | 63 | } |
64 | if (!cache_exist(item)) | 64 | if (!cache_exist(item)) { |
65 | cgit_fill_cache(item); | 65 | cgit_fill_cache(item); |
66 | cache_unlock(item); | 66 | cache_unlock(item); |
67 | } else { | ||
68 | cache_cancel_lock(item); | ||
69 | } | ||
67 | } else if (cache_expired(item) && cache_lock(item)) { | 70 | } else if (cache_expired(item) && cache_lock(item)) { |
68 | if (cache_expired(item)) | 71 | if (cache_expired(item)) { |
69 | cgit_fill_cache(item); | 72 | cgit_fill_cache(item); |
70 | cache_unlock(item); | 73 | cache_unlock(item); |
74 | } else { | ||
75 | cache_cancel_lock(item); | ||
76 | } | ||
71 | } | 77 | } |
72 | } | 78 | } |
73 | 79 | ||
74 | static void cgit_print_cache(struct cacheitem *item) | 80 | static void cgit_print_cache(struct cacheitem *item) |
75 | { | 81 | { |
76 | static char buf[4096]; | 82 | static char buf[4096]; |
77 | ssize_t i; | 83 | ssize_t i; |
78 | 84 | ||
79 | int fd = open(item->name, O_RDONLY); | 85 | int fd = open(item->name, O_RDONLY); |
80 | if (fd<0) | 86 | if (fd<0) |
81 | die("Unable to open cached file %s", item->name); | 87 | die("Unable to open cached file %s", item->name); |
82 | 88 | ||
83 | while((i=read(fd, buf, sizeof(buf))) > 0) | 89 | while((i=read(fd, buf, sizeof(buf))) > 0) |
84 | write(STDOUT_FILENO, buf, i); | 90 | write(STDOUT_FILENO, buf, i); |
85 | 91 | ||
86 | close(fd); | 92 | close(fd); |
87 | } | 93 | } |
88 | 94 | ||
89 | int main(int argc, const char **argv) | 95 | int main(int argc, const char **argv) |
90 | { | 96 | { |
91 | struct cacheitem item; | 97 | struct cacheitem item; |
92 | 98 | ||
93 | cgit_read_config("/etc/cgitrc", cgit_global_config_cb); | 99 | cgit_read_config("/etc/cgitrc", cgit_global_config_cb); |
94 | cgit_querystring = xstrdup(getenv("QUERY_STRING")); | 100 | cgit_querystring = xstrdup(getenv("QUERY_STRING")); |
@@ -45,42 +45,43 @@ extern char *cgit_query_page; | |||
45 | extern char *cgit_query_head; | 45 | extern char *cgit_query_head; |
46 | extern char *cgit_query_sha1; | 46 | extern char *cgit_query_sha1; |
47 | 47 | ||
48 | extern int htmlfd; | 48 | extern int htmlfd; |
49 | 49 | ||
50 | extern void cgit_global_config_cb(const char *name, const char *value); | 50 | extern void cgit_global_config_cb(const char *name, const char *value); |
51 | extern void cgit_repo_config_cb(const char *name, const char *value); | 51 | extern void cgit_repo_config_cb(const char *name, const char *value); |
52 | extern void cgit_querystring_cb(const char *name, const char *value); | 52 | extern void cgit_querystring_cb(const char *name, const char *value); |
53 | 53 | ||
54 | extern char *fmt(const char *format,...); | 54 | extern char *fmt(const char *format,...); |
55 | 55 | ||
56 | extern void html(const char *txt); | 56 | extern void html(const char *txt); |
57 | extern void htmlf(const char *format,...); | 57 | extern void htmlf(const char *format,...); |
58 | extern void html_txt(char *txt); | 58 | extern void html_txt(char *txt); |
59 | extern void html_attr(char *txt); | 59 | extern void html_attr(char *txt); |
60 | extern void html_link_open(char *url, char *title, char *class); | 60 | extern void html_link_open(char *url, char *title, char *class); |
61 | extern void html_link_close(void); | 61 | extern void html_link_close(void); |
62 | 62 | ||
63 | extern int cgit_read_config(const char *filename, configfn fn); | 63 | extern int cgit_read_config(const char *filename, configfn fn); |
64 | extern int cgit_parse_query(char *txt, configfn fn); | 64 | extern int cgit_parse_query(char *txt, configfn fn); |
65 | 65 | ||
66 | extern void cache_prepare(struct cacheitem *item); | 66 | extern void cache_prepare(struct cacheitem *item); |
67 | extern int cache_lock(struct cacheitem *item); | 67 | extern int cache_lock(struct cacheitem *item); |
68 | extern int cache_unlock(struct cacheitem *item); | 68 | extern int cache_unlock(struct cacheitem *item); |
69 | extern int cache_cancel_lock(struct cacheitem *item); | ||
69 | extern int cache_exist(struct cacheitem *item); | 70 | extern int cache_exist(struct cacheitem *item); |
70 | extern int cache_expired(struct cacheitem *item); | 71 | extern int cache_expired(struct cacheitem *item); |
71 | 72 | ||
72 | extern char *cgit_repourl(const char *reponame); | 73 | extern char *cgit_repourl(const char *reponame); |
73 | extern char *cgit_pageurl(const char *reponame, const char *pagename, | 74 | extern char *cgit_pageurl(const char *reponame, const char *pagename, |
74 | const char *query); | 75 | const char *query); |
75 | 76 | ||
76 | extern void cgit_print_error(char *msg); | 77 | extern void cgit_print_error(char *msg); |
77 | extern void cgit_print_docstart(char *title, struct cacheitem *item); | 78 | extern void cgit_print_docstart(char *title, struct cacheitem *item); |
78 | extern void cgit_print_docend(); | 79 | extern void cgit_print_docend(); |
79 | extern void cgit_print_pageheader(char *title); | 80 | extern void cgit_print_pageheader(char *title); |
80 | 81 | ||
81 | extern void cgit_print_repolist(struct cacheitem *item); | 82 | extern void cgit_print_repolist(struct cacheitem *item); |
82 | extern void cgit_print_summary(); | 83 | extern void cgit_print_summary(); |
83 | extern void cgit_print_log(const char *tip, int ofs, int cnt); | 84 | extern void cgit_print_log(const char *tip, int ofs, int cnt); |
84 | extern void cgit_print_view(char *hex); | 85 | extern void cgit_print_view(char *hex); |
85 | 86 | ||
86 | #endif /* CGIT_H */ | 87 | #endif /* CGIT_H */ |