Re: lfs (CVS version) compilation error under Ununtu-9.04 Jaunty

[Edgar Toernig <froese@...>]

Fabio Mascarenhas wrote:
>
> I think mkstemp and symlink are supposed to
> be atomic, so where is the race condition?

The symlink is fine.  The race is when the lock is broken:

	/////
	while (symlink("foobar", lockfile) == -1) {
		if (errno == EEXIST)
		  if (lstat(lockfile, &st) == 0)
		    if (time(NULL) - st.st_mtime > expire) {
		      unlink(lockfile)
		      continue;
		    }
		return ERROR
	}
	return SUCCESS;
	/////

	Process A			Process B

  T	symlink -> EEXIST		symlink -> EEXIST
  i	lstat -> OK			lstat -> OK
  m	expired				expired
  e	unlink
	symlink -> OK
	return SUCCESS 
					unlink
					symlink -> OK
					return SUCCESS

Generally, breaking locks without appropriate rollback is
seldom a good idea.  The lock holder tried to perform some
non-atomic changes (else it wouldn't need the lock) and
for whatever reason didn't finish with that.  Breaking the
lock now just means: I don't care if the data is in an
inconsistant state.  So, why using a lock in the first
place? *g*

I was wrong about the busy loop, sorry.  When I read the code,
I thought the "while" loops until it gets the lock.

The memory and fd leak happens when lua_newuserdata fails.

Regarding that "strange algorithm": I have no idea what the
mkstemp file is good for.  Afaics, it has no function.
Its name is used as the contents of the symlink and it is
kept open until unlock.  What for?

> If the lock is taken lock_dir returns an error, how to wait until trying
> again is up to the user.

Well, here is a global problem with lfs: error reporting.
You only get a random (system and locale dependant) error
string.  The caller has no chance to check what kind of
error happened (i.e. why was the lock not taken? ENOENT?
EACCESS? EEXIST?) and take appropriate action.

Btw, errno is often read too late in lfs, intervening calls
may have modified it.

Ciao, ET.