[ditrack commit] r1638 - in src/trunk/tests: . lock lock/low_level_lock.tc

[Ivan Glushkov]

On 6/5/07, Vlad Skvortsov <vss at 73rus.com> wrote:
> gli at ditrack.org wrote:
> > Author: gli
> > Date: 2007-06-01 13:52:06 -0700 (Fri, 01 Jun 2007)
> > New Revision: 1638
> >
> > Added:
> >    src/trunk/tests/lock/
> >    src/trunk/tests/lock/low_level_lock.tc/
> >    src/trunk/tests/lock/low_level_lock.tc/test.py
> > Log:
> > fixing i#52. added new testcase for low level lock
> >
> > Added: src/trunk/tests/lock/low_level_lock.tc/test.py
> > ===================================================================
> > --- src/trunk/tests/lock/low_level_lock.tc/test.py                            (rev 0)
> > +++ src/trunk/tests/lock/low_level_lock.tc/test.py    2007-06-01 20:52:06 UTC (rev 1638)
> >
> >
> [skipped]
>
> > +import time
> > +
> > +if not os.name == "posix":
> >
>
> if os.name != "posix"? :-)
>
> > +    sys.exit(0)
> > +
> > +
> > +import DITrack.Util.Locking
> > +import DITrack.DB.Exceptions
> >
>
> Why do you need DITrack database exceptions to test low level locking
> primitives? Looks like a layering violation.
yeap.
fixed in r1647.

>
> > +
> > +childslist = []
> > +lockfile = "LOCK"
> > +
> > +if os.path.exists(lockfile):
> > +    os.unlink(lockfile)
> > +
> > +def new_child(mode, sleep_after_signal=0):
> > +    (p_r, p_w) = os.pipe()
> > +    pid_new = os.fork()
> > +    if (pid_new == 0):
> > +        lock = DITrack.Util.Locking.lock(open(lockfile,"w"), mode)
> >
>
> Style?
>
> > +        os.close(p_w)
> > +        os.read(p_r,1)
> >
>
> Style?
>
> > +        if (sleep_after_signal):
> > +            time.sleep(sleep_after_signal)
> > +        lock.unlock()
> > +        os._exit(0)
> > +    elif (pid_new < 0):
> > +        sys.stderr.write("Can't fork\n")
> > +        sys.exit(1)
> > +    os.close(p_r)
> > +    return (pid_new, p_w)
> > +
> > +for i in range(3):
> > +    (pid_new, p_w) = new_child("r")
> > +    childslist.append((pid_new, p_w))
> > +
> > +time.sleep(1)
> >
>
> Possible race condition: you should rather wait until all children write
> something to their pipes.

hmm.
I began to use pipes just in order to synchronize childs and parent
(in the both directions). Apparently i forgot to use it here.
fixed in r1644


>
> > +
> > +sys.stdout.write("Test for several shared locks: ")
> >
>
> stderr?
>
> > +for (child_pid, child_pipe) in childslist:
> > +    try:
> > +        lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "w")
> >
>
> Style?
>
> > +    except DITrack.DB.Exceptions.DBIsLockedError:
> > +        pass
> > +    else:
> > +        sys.stdout.write("FAILED\n")
> >
>
> stderr?
>
> > +        sys.exit(1)
> > +    os.write(child_pipe, "a")
> > +    os.waitpid(child_pid, 0)
> > +sys.stdout.write("OK\n")
> >
>
> stderr?
>
> > +
> > +sys.stdout.write("Test for exclusive lock: ")
> >
>
> stderr?
>
> > +lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "w")
> >
>
> Style?
>
> > +try:
> > +    lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "r")
> >
>
> Style?
>
> > +except DITrack.DB.Exceptions.DBIsLockedError:
> > +    pass
> > +else:
> > +    sys.stdout.write("FAILED\n")
> >
>
> stderr?
>
> > +    sys.exit(1)
> > +
> > +try:
> > +    lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "w")
> > +except DITrack.DB.Exceptions.DBIsLockedError:
> > +    pass
> > +else:
> > +    sys.stdout.write("FAILED\n")
> >
>
> stderr?
>
> > +    sys.exit(1)
> > +lock.unlock()
> > +sys.stdout.write("OK\n")
> > +
> > +
> > +sys.stdout.write("Test 1 for timeout on lock: ")
> > +(child_pid, child_pipe) = new_child("w", sleep_after_signal=4)
> > +os.write(child_pipe, "a")
> > +try:
> > +    lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "w", timeout=1)
> >
>
> Style?
>
> > +except DITrack.DB.Exceptions.DBIsLockedError:
> > +    sys.stdout.write("OK\n")
> >
>
> Style?
>
> > +    pass
> >
>
> 'pass' is redundant here.
>
> > +else:
> > +    sys.stdout.write("FAILED\n")
> >
>
> Stderr?
>
> > +    sys.exit(1)
> > +
> > +sys.stdout.write("Test 2 for timeout on lock: ")
> >
>
> Stderr?
>
> > +try:
> > +    lock = DITrack.Util.Locking.lock(open(lockfile,"w"), "w", timeout=3)
> >
>
> Why would '3' work here? Subject to another race condition, as far as I
> can see.
> > +except DITrack.DB.Exceptions.DBIsLockedError:
> > +    sys.stdout.write("FAILED\n")
> >
>
> Stderr?
>
> > +    sys.exit(1)
> > +sys.stdout.write("OK\n")
> >
>
> Stderr?
>

Style changes commited in r1643, r1646, r1648

Now about stderr.
I suppose stderr should be used only in extraodinary situations (for
example, all non-excepted-exceptions), but not for all output.
I agree, changes are needed for the test to write out some diagnostic
to stderr when error occurs.
fixed in r1645

Ivan.