[ruby-talk:02058] Lockfiles

David Douthitt writes:
> | >>> Clemens Hintze <[e-mail]> 03/20/00 11:08PM >>>

...

> | Perhaps it is what you have searched?
> | 
> | But then, nobody has told me if you like to re-invent the wheel again
> | ;-)
> 
> Only sometimes :-)

And here you have found the *great* danger of Ruby... because
programming in Ruby is so fun, and beause Ruby makes it really easy to
re-invent the wheel again and again, I often do it! ;-)

Instead of using smart code snippets from other, I really like to
write them myself. Not every time, but if I am bored, for example. I
never had that feeling with other languages... :-)

> Yet, I don't think your script is what I had in mind.  I'm not looking
> to "lock a file" - but to create a UNIX-style lockfile in
> (typical example) /var/spool/locks/ - as it is under HP-UX 10.20 here.
> This looks like your class LockFile but your code seems quite complex.
> I'm not sure that all that is what I needed.  My class Lock turned out
> to be much simpler I think.

If you only want to synchronize your own apps, you do not need the
whole complexity from my filelock.rb. But if you wants to work
together with the e.g. system mailbox, things are more complicate
then. Then you would also have to check if lockfile was stolen and
such things.

> 
> That first line is a marvel!  "/bin/env ruby" ... hmmm!

You do not like it? ;-) I use it anytime! The program 'env' is used
for two purposes.

  1. Displaying of all environment variable (called without parameter)
  2. Start a program with some environment variables set to a certain
     value *only* for that process.

The 2nd point is funny! Suppose you have your DISPLAY variable set to
qiao:0, but you will display an xterm on didi:0, you can do this:

   env DISPLAY=didi:0 xterm -title "Yuhu2" &

And now, *only* for the xterm process DISPLAY was set to another
value. The nice side effect is, that the program 'env' will search for 
the executable to be executed in your PATH. So '/bin/env ruby' means:
search the program ruby in along PATH. If found start it. So my
scripts becomes location independend from the Ruby interpreter.

Unfortunately the 'env' programm can be found sometimes in '/bin'
sometimes in '/usr/bin'. But there are much more locations possible
for Ruby. At home I have a link from '/usr/bin/env' -->
'/bin/env'. That makes me really independend! :-)))

But now coming to your script... May I propose some changings?
Proposing them does not mean your solution is wrong, however!

> Here's my code:
> 
> #!/opt/ruby/bin/ruby
> 
> class Lock
>    attr_accessor :locked_up, :lockdir, :lockfile
> 
>    def initialize (lockf = nil)
>       @locked_up = false
>       @lockdir = "/var/spool/locks"

Although Ruby does not really need it because it handles '/' as
separator all the time, I think it would be better to use File::join
to do the job:

        @lockdir = File::Separator + File.join(%w(var spool locks)) 

>       if (lockf == nil)
>          @lockfile = (@lockdir + "/" + File.basename($0) + ".lock")

Here too...
!!!        @lockfile = File.join(@lockdir, File.basename($0)) + ".lock"

>       else
>          @lockfile = (@lockdir + "/" + lockf + ".lock")

And here
!!!        @lockfile = File.join(@lockdir, lockf) + ".lock"

>       end
>    end
> 
>    def setlock (lockf = @lockfile)
>       raise "Lockfile not set!" if lockf == nil;
>       raise "Lock failed!" if
>          ! system("set -o noclobber ; cat /dev/null 2> /dev/null > #{lockf}")

Why do you use such complicate system call here? I would prefer a Ruby 
solution:
        if not File.exist?(lockf)
           begin File.open(lockf, "w") { |fp| fp.write(Process.pid) }
           rescue
              raise "Lock failed (system error)"
           end
        else
           raise "Lock failed (already locked)"
        end

Is a little bit longer but portable and deliver two different
exceptions depending on the error occured. Okay not different
exceptions but different explanations ;-)

> 
>       at_exit { self.unlock }
>       @locked_up = true
>    end
> 
>    def locked?
>       test(?e, @lockfile)

Is ok! But I find the following more self descriptive...
!!!     File.exist? @lockfile

>    end
> 
>    def Lock.locked? (lockf)
>       test(?e, "/var/spool/locks/" + lockf)

Same as above...
!!!     File.exist?(File::Separator + File.join(%w(var spool locks)) + lockf
>    end
> 
>    def locked
>       self.setlock
>       yield
>       self.unlock
>    end
> 
>    def unlock
>       test(?e, @lockfile) if
>          raise "Unlock failed!" if
>             ! system("rm -f #{@lockfile}");

What you are try to doing here? If 'rm -f' failed the exception is
raised and 'test(?e,...)' will not performed. If 'rm' went well,
neither 'raise' nor 'test' will be executed.

        raise "Unlock failed (no lock)" unless File.exist?(@lockfile)
        begin
          File.delete @lockfile
        rescue
          raise "Unlock failed (couldn't remove lockfile)"
        end

> 
>       @locked_up = false
>    end
> end
> 
> >>> I use it this way:
> 
> mylock = Lock.new
> 
> mylock.locked {
> #  ....do stuff....
>    }
> 
> >>> or this way:
> 
> return if Lock.locked?("Oracle.lock")
> 
> >>>
> 
> What do you think?

Your code is ok! Only there are some things I would like to
remark. 

   1. You often use constants like "/var/spool/locks" throughout your
      whole code.
   2. You use too much system dependend things like system(...)
      here. This it not really necessary.
   3. You should not use things like 'test(?e, ...)'. The Ruby classes 
      are more descriptive.
   4. IMHO, your code is not carefully designed for the class/instance 
      clash. Means, you do some things both in class methods and in
      instance methods. 
   5. You should not on any case define accessors for your class. I
      would consider it not-so-good style if I would have to:

      lock = Lock.new("/var/spool/mail/cle")
      lock.lockdir = "/var/spool/mail"
      lock.setlock

      following would be better, IMO:

      lock = Lock.new("/var/spool/mail/cle", "/var/spool/mail")
      lock.setlock
       
All in all I would say, that your code may be appropiate for what you
have in mind for your apps. But it is not generic enough, IMHO! No
steal-the-lock feature. No wait for the lock. No auto-unlock as soon
as I do not need this file instance anymore...

But keep fun, ok? :-)

\cle

-- 
Clemens Hintze  mailto: [e-mail]