From: Wanlong G. <gao...@cn...> - 2012-05-28 02:27:48
|
On 05/28/2012 02:19 AM, Garrett Cooper wrote: > On Sun, May 27, 2012 at 2:06 AM, Wanlong Gao <gao...@cn...> wrote: >> On 05/27/2012 04:23 PM, Garrett Cooper wrote: >> >>> On Sun, May 27, 2012 at 1:14 AM, Wanlong Gao <gao...@cn...> wrote: >>>> On 05/27/2012 04:00 PM, Garrett Cooper wrote: >>>> >>>>> On Sun, May 27, 2012 at 12:33 AM, Wanlong Gao <gao...@cn...> wrote: >>>>>> On 05/26/2012 10:15 PM, Garrett Cooper wrote: >>>>>> >>>>>>> The testcase tests to see whether or not locks are successfully >>>>>>> inherited across forking processes, as the requirements for fork state >>>>>>> that they should not be. The problem is that the test tests the negative >>>>>>> case for ftrylockfile (!= 0) instead of the positive case, which creates >>>>>> >>>>>> The ftrylockfile() function returns zero for success (the lock was obtained), >>>>>> and nonzero for failure. >>>>>> So I think the origin is right. >>>>> >>>>> My description might be wrong, but the fix is right. ftrylockfile >>>>> should fail in the testcase (not succeed) because the file lock isn't >>>>> owned by the forked process. It succeeds simply because the delay is >>>> >>>> >>>> But on my system ftrylockfile() always return 0, and the test PASSED. >>>> and with your patch, it fails. >>> >>> Then that's a Linux/POSIX bug then. From fork(2) on OpenGroup [1]: >>> >>> "File locks set by the parent process are not inherited by the child process." >>> >>> And from flockfile(2) on OpenGroup [2]: >>> >>> " The flockfile() function is used by a thread to acquire ownership >>> of a ( FILE *) object. >>> >>> The ftrylockfile() function is used by a thread to acquire >>> ownership of a ( FILE *) object if the object is available; >>> ftrylockfile() is a non-blocking version of flockfile(). >>> >>> ... >>> >>> None for flockfile() and funlockfile(). The function >>> ftrylockfile() returns zero for success and non-zero to indicate that >>> the lock cannot be acquired." >> >>> >> >>> The goal/algorithm as stated at the top of the testcase was... >>> >>> "* The steps are: >>> * -> lock stdout >>> * -> fork >>> * -> child creates a thread >>> * -> child thread trylock stdout >>> * -> join the child >>> >>> * The test fails if the child thread cannot lock the file >>> * -- this would mean the child process got stdout file lock ownership." >> >> >> Yes, if trylock can't get the lock, it should return nonzero, I think >> the concept is right. >> But you mean if trylock() can't get the lock, it should return zero? > > No. > > Looking into the implementation of ftrylockfile further, it looks like > the locking is not advisory locking in the traditional sense, but > instead thread-based locking -- this matches the description in the > FreeBSD and OpenGroup manpage. > > The problem that I see is multi-fold: > 1. The testcase's original test for success was right as the testcase > is using fork, not forkall. > 2. There's a bug in FreeBSD that I need to investigate and fix. > > I think that the testcase should test another stream than stdout > though (say, open up /dev/null via fopen?). It would at least allow > the test to return immediately instead of blocking indefinitely. > > Do you object to the other changes? I think I have no more objection. Thanks, Wanlong Gao > > Thanks! > -Garrett > |