Merge lp:~invidian/intltool/fix-dangling-lock-file into lp:intltool

Proposed by Mateusz Gozdek
Status: Needs review
Proposed branch: lp:~invidian/intltool/fix-dangling-lock-file
Merge into: lp:intltool
Diff against target: 19 lines (+2/-0)
1 file modified
intltool-merge.in (+2/-0)
To merge this branch: bzr merge lp:~invidian/intltool/fix-dangling-lock-file
Reviewer Review Type Date Requested Status
Bernhard M. Wiedemann (community) Approve
intltool Developers Pending
Review via email: mp+400825@code.launchpad.net

Description of the change

Recently added https://bazaar.launchpad.net/~intltool/intltool/trunk/revision/748 right now leaves lock file after creating it, which breaks "distcheck" for some packages e.g. https://bugs.archlinux.org/index.php?do=details&action=details.addvote&task_id=67098.

This commit fixes that by unlinking the lock file after use.

To post a comment you must log in.
Revision history for this message
Mateusz Gozdek (invidian) wrote :

I tested it locally on some builds and it works fine, but I can't confirm that it does not introduce a regression in race for which lock was originally added.

Revision history for this message
Bernhard M. Wiedemann (ubuntubmw) wrote :

In this case, it is probably OK to remove the lock file, since it is a one-time race.
After the first cache-writer is done, any amount of cache-readers can use the fully created caches.

The original problem was that cache-readers started to use partial caches before the cache-writer was done writing.

review: Approve
Revision history for this message
Dan Nicholson (danbnicholson) wrote :

> In this case, it is probably OK to remove the lock file, since it is a one-
> time race.
> After the first cache-writer is done, any amount of cache-readers can use the
> fully created caches.
>
> The original problem was that cache-readers started to use partial caches
> before the cache-writer was done writing.

I have to disagree here. flock works at the inode level, so when you delete the lock file, the next process to acquire the lock will open a new file and try to lock a different inode. We did this exact thing at Endless and it immediately broke. You really want the lock file to live as long as possible.

I think the correct fix is just to update the Makefile.in.in so that it cleans the lock file. Then you know at least all the processes make launched are done.

Unmerged revisions

749. By Mateusz Gozdek

intltool-merge.in: remove lock file after locking

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'intltool-merge.in'
2--- intltool-merge.in 2017-05-18 19:09:18 +0000
3+++ intltool-merge.in 2021-04-08 17:37:42 +0000
4@@ -401,6 +401,7 @@
5 if ($cache_file_age <= &get_newest_po_age)
6 {
7 close($lockfh);
8+ unlink("$cache_file.lock");
9 &load_cache;
10 return;
11 }
12@@ -409,6 +410,7 @@
13
14 &create_cache;
15 close($lockfh);
16+ unlink("$cache_file.lock");
17 }
18
19 sub add_translation

Subscribers

People subscribed via source and target branches