Delete lock file if contained pid is stale #9
Conversation
a3f
force-pushed
the
delete-stale-lockfile
branch
from
3982c7f to
cd4c31c
Compare
Previously, a different process could create the lock between both calls to open(2) with the result that two processes access the same serial port. Fix this by only opening the lock file with O_CREAT | O_EXCL. Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
This is the format specified by the FHS[1]. While at it remove the now outdated comment about kermit compatibility. It no longer applies to ckermit, which now follows the HDB UUCP format as well[2]. [1]: http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES [2]: pengutronix#9 (comment) Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
If microcom crashes, e.g. due to the buffer underflow described in pengutronix#10, it may leave a stale lock file behind. Teach microcom detection of these stale lock files. Note that this is racy, two processes may detect a stale lock file and one might unlink the other's lock. Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
|
I've changed microcom to use the HDB UUCP format for its lock files. Deleting stale lock files though is racy and I think it can't be fixed without file locks, but as it was racy before anyway, it might still be good enough for inclusion :-) |
While at it remove the now outdated comment about kermit compatibility. It no longer applies to ckermit [1]. [1]: pengutronix#9 (comment) Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
While at it remove the now outdated comment about kermit compatibility. It no longer applies to ckermit [1]. [1]: pengutronix#9 (comment) Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
|
Any comments on this? |
| if (opt_force) { | ||
| printf("lockfile for port exists, ignoring\n"); | ||
| serial_unlock(); | ||
| goto relock; |
There was a problem hiding this comment.
if serial_unlock() fails to delete the lockfile, this is an endless loop.
| do { | ||
| ret = read(fd, &pidbuf[nbytes], sizeof(pidbuf) - nbytes - 1); | ||
| nbytes += ret; | ||
| } while (ret > 0 && nbytes < sizeof (pidbuf) - 1); |
There was a problem hiding this comment.
Would it make sense here to do some additional checks on the format? E.g. check you read the file completely? If the lockfile happens to be created by an older microcom with pid 33333333 you check for pid 3333 instead. (I think, didn't test.)
| if (ret == 1 && kill(pid, 0) < 0 && errno == ESRCH) { | ||
| printf("lockfile contains stale pid, ignoring\n"); | ||
| serial_unlock(); | ||
| goto relock; |
There was a problem hiding this comment.
same issue as with the first patch in this series (i.e. endless loop if serial_unlock fails to remove lockfile)
|
With move to flock, this is no longer applicable. |
2 participants
Fixes #2.