Landed in r.3051

A deadlock is NOT a relative timeout waiting on a lock, but rather a state where two threads (or more) are locking opposite resources and each is waiting for the other one to release it.

Finding deadlocks is not easy, and there are programs to do that (see TypeMock Racer at http://www.typemock.com/learn_about_typemock_racer.html).

You can easily make a lock wait more than any arbitrary amount of time, but still not be a deadlock.
Hence, the name of the option is misleading; It should be LOCK_TIMEOUT, which is both more correct and more intuitive.

I'm not sure this feature really should be included as a builtin part of Boo. It's relatively easy for anyone to define a macro (actually, even a meta-method) that locks with a timeout, and use that instead of the normal lock macro.

Even if it is included, it should certainly not be on by default. Additionally, I don't think it should be a compilation symbol, but rather something like an assembly attribute, like [assembly: LockTimeout(1000)].

>A deadlock is NOT a relative timeout waiting on a lock, but rather a state where two threads (or more) are locking opposite resources and each is >waiting for the other one to release it.

Hmm yeah, that is what the testcase is about.

>Finding deadlocks is not easy, and there are programs to do that

It's much easier and requires no special tools (that are not always available) to throw an exception when a lock is waiting too long.

> You can easily make a lock wait more than any arbitrary amount of time, but still not be a deadlock.

Yeah but in practice you use lock: only for small amount of time, else you more use a Mutex/WaitHandle/*ResetEvent.
And that arbitrary amount of time to consider a wait a deadlock is configurable.

> Hence, the name of the option is misleading; It should be LOCK_TIMEOUT, which is both more correct and more intuitive.

I agree, LOCK_TIMEOUT sounds better !

> I'm not sure this feature really should be included as a builtin part of Boo. It's relatively easy for anyone to define a macro (actually, even a meta-method) that locks with a timeout, and use that instead of the normal lock macro.

Of course it is easy to define a macro, lock is already a macro for that matter. THe problem being that you would have to replace all lock:
in your code by your special macro instead of using the builting lock: doesnt sound practical to me.

> Even if it is included, I don't think it should be a compilation symbol, but rather something like [assembly:LockTimeout(1000)]

Which requires modifying the sources, rather than being able to change the build settings in nant/make or whatever, so I dont think this is practical either.

> Hmm yeah, that is what the testcase is about.
> It's much easier and requires no special tools (that are not always available) to throw an exception when a lock is waiting too long.

That's true, but then it's not a deadlock detector but a lock timeout detector that serves as a heuristic for finding deadlocks. It's misleading of Boo to refer to lock timeouts as deadlocks – they might be possible deadlocks, or it might be that a lock is taking to much time because of other reasons. So the feature should really be "automatic exceptions on lock timeout" rather than "deadlock detector", and that should be represented in the name of the symbol, the text of the exception, etc.

> Yeah but in practice you use lock: only for small amount of times, else you more use a Mutex/WaitHandle.

Why? More importantly, do you really think that's how everybody will use the lock macro in Boo?

> Of course it is easy to define a macro, lock is already a macro for that matter. The problem being that you would have to replace all lock:
in your code by your special macro instead of using the builting lock: doesnt sound practical to me.

Trading unlimited locks for timed locks is not something you do "on the fly" and outside your source. If the timed locks throw an exception, you'd want to modify your sources to catch that exception and do something with it. You'd want to change the way you use lock macros to take into account the possibility of a lock timeout exception. I don't see what value is gained from recompiling an assembly with timed locks without handling the timeouts.

What I suggest is this:
1. The feature should be named "lock timeout" rather than "deadlock detector" (as you agreed).

2. The feature should be activated by an assembly attribute rather than a define symbol (as you disagreed).

3. The exception thrown must be a meaningful one! System.ApplicationException("DEADLOCK DETECTED") is useless if you want to handle the exception or understand where it comes from.
We should throw a System.TimeoutException with an appropriate message ("Timeout expired trying to acquire lock on '$($lockExpression.ToCodeString)'." or something like that). That way users can catch TimeoutExceptions when using locks, and if they don't, they get a meaningful message.

4. Maybe the lock macro should accept a second parameter that would define the timeout (regardless of the auto-timeout feature), and the auto-timeout feature will just act as though the user specified that timeout whenever an untimed lock macro is used (I'm not sure about this one).

>That's true, but then it's not a deadlock detector but a lock timeout detector that serves as a heuristic for finding deadlocks. It's misleading of Boo to refer to lock timeouts as deadlocks - they might be possible deadlocks, or it might be that a lock is taking to much time because of other reasons.

Hmm.. I'd be interested to know what these "reasons" would be ? A lock timeouts because it cannot be acquired, why cannot it be acquird? because it has not been released by another resource (lock)... hence a deadlock CQFD.
My approach is well discussed here :
http://www.interact-sw.co.uk/iangblog/2004/03/23/locking
But of course in C# you cannot redefine the behavior of the lock() builtin, so they have to create and use a special timedlock...

>> Yeah but in practice you use lock: only for small amount of times, else you more use a Mutex/WaitHandle.
>Why? More importantly, do you really think that's how everybody will use the lock macro in Boo?

Hmm sorry but I have yet to see any code (in Boo/C# or whatever) that uses a lock statement for a huge amount of time... if you know a program that does that I'd be interested to know which one...
And anyways even if there is some crazy people doing that, well LOCK_TIMEOUT would not be useful to them that's all, no harm done, their 'exotic' code wont feel any impact.

>1. The feature should be named "lock timeout" rather than "deadlock detector" (as you agreed).
Ack. Though the only sensible usage of this is to detect dead locks... but yeah LOCK_TIMEOUT is a better name.

>2. The feature should be activated by an assembly attribute rather than a define symbol (as you disagreed).
And I still disagree. What is the advantage of having it as an assembly attribute instead of a symbol.
With a symbol you can simply define a "debug locks" target to build and find the locks that are causing you problems without editing manually the code.

>3. The exception thrown must be a meaningful one! System.ApplicationException("DEADLOCK DETECTED") is useless if you want to handle the >exception or understand where it comes from.
>We should throw a System.TimeoutException with an appropriate message ("Timeout expired trying to acquire lock on '>$($lockExpression.ToCodeString)'." or something like that). That way users can catch TimeoutExceptions when using locks, and if they don't, they >get a meaningful message.

I agree, though you seem to overlook that you cannot do typical exception handling with threads so anyways the exception have to be catched by the debugger or through a ThreadExceptionEventHandler, hence as "meaningless" you think ApplicationException is just enough.
Again I agree but that's just cosmetic.

>4. Maybe the lock macro should accept a second parameter that would define the timeout (regardless of the auto-timeout feature), and the auto-timeout feature will just act as though the user specified that timeout whenever an untimed lock macro is used (I'm not sure about this one).

lock macro already accepts multiple parameters, so that's not possible, and I think it would be of little value anyway, if you need a timed lock you are certainly doing something wrong in the first place.

Hmm by the way I forgot to comment on the probably most important part :

??
Trading unlimited locks for timed locks is not something you do "on the fly" and outside your source. If the timed locks throw an exception, you'd want to modify your sources to catch that exception and do something with it. You'd want to change the way you use lock macros to take into account the possibility of a lock timeout exception. I don't see what value is gained from recompiling an assembly with timed locks without handling the timeouts.
??

I guess you have a misunderstanding about what this is all about.

The goal is not to handle timeouts, the goal is to find deadlocks (ie. locks that cannot be acquired).
Timed locks (there is no such thing actually) are not locks that expires if the execution takes too long time, it expires if it cannot be acquired in a certain amount of time, that's a good reason the method used is named Monitor.*Try*Enter.
You don't want to change the way lock macros are used, this is a feature only for debugging deadlocks ; you don't want to take into account the possibility of a timeout exception when trying to acquire a lock, simply because if you use the lock construct it means you want to protect a critical-region of code from concurrent access, that's all.
If you cannot enter the region, the program typically deadlocks... there is no safe way around it.
If you want your program to do some error recovery about it (despite deadlocks being a design/compile-time defect) then you do not want to use lock construct in the first place but another synchronization mechanism instead (or in addition to).

Locks can take a lot of time for reasons not related to threading, but rather because the work that is performed inside the lock takes a lot of time (for example: database querying, network / file system operations, long calculations etc.) – indeed it's not a good practice to lock on such actions, but it's still possible and sometimes unavoidable due to design reasons, and sometimes it isn't apparent (for example, you wouldn't think reading a file from disk will timeout your lock, but what if the file is on a network share? or maybe your AOP-logging function uses a database?).

Of course you can do typical exception handling with threads, since the thread that issues the lock statement can also catch the exception:

def AutonomousMethodRunningInSomeThread():
  try:
    lock SomeResource:
      # do stuff
  catch ex as TimeoutException:
     # handle a timeout, possibly communicate problem to other threads

Using a TimeoutException is more than just cosmetic, in fact it is mandated if you want to handle the exception inside the waiting thread, which I believe is preferred over using a ThreadException event.

> if you need a timed lock you are certainly doing something wrong in the first place.
Huh? Then why are we considering built-in support for doing that "something wrong"?

>Locks can take a lot of time for reasons not related to threading,... IO stuff...

What you describe is typically a horrible way to design multi-threaded software... really if you know any program that uses a lock around I/O operations I'd be interested to know which...
At least if it is used somewhere then by-design that method would not be useable by any other thread.

>Huh? Then why are we considering built-in support for doing that "something wrong"?

I guess you have not yet read my comment just above.
Because it is not a runtime feature, only a debugging feature.

I will summarize my point:

Acquiring a lock can time out because of several reasons:
1. Because there is a deadlock by-design, and the lock can never be acquired.
2. Because another thread is holding the lock for longer than the timeout period, but will eventually release it and there will be no deadlock.

Option #2 can happen if the work inside the lock is long (bad practice but possible) or if the lock timeout is too short (always possible due to user specifying a short timeout).
Nobody said that locks are always placed around "immediate" actions (despite this being the recommended approach), and yes, I've seen programs use locks around longer portions.

You wish to introduce a debugging option that reports a deadlock for both of these options. I simply wish to say that such a feature should be aware of the possibility of false positives (such as scenario #2) and act appropriately, saying "Timeout expired acquiring a lock - possible deadlock" rather than "DEADLOCK DETECTED!".

> It is possible and probable to use a timed lock in this manner, and in this scenario, I do want to take into account the possibility of a timeout acquiring the lock, and I can do some error recovery about it.

Exactly... and so if you want do this what would you be using ? not lock: [since by definition a lock section entry does not timeout]
So that case is completely unrelated and I think that is where the misunderstanding is.

> Even if I don't use timed locks as a design decision but only as a debugging tool, acquiring the lock could still time out despite there being no deadlock, exactly as I described - only a wrong estimation of the lock timeout.

And that's why the lock timeout is configurable in the first place indeed...

Since you commented let me follow up on Option #2, that you agreed, is, bad practice

The good thing is that this feature would still help to detect this bad practice !

Updated the issue description with the new symbol name and clarifying the last sentence.

I think having the ability to change every lock application to have a timeout is a good thing.

Using a define is like using a global variable, there's no way to control scope.