[jcifs] Deadlock while cleaning up old sessions in transport

Felix Schumacher felix.schumacher at internetallee.de
Tue Aug 14 12:30:55 GMT 2007


Am Mo, 13.08.2007, 21:07, schrieb Michael B Allen:
> On Mon, 13 Aug 2007 17:17:52 +0200 (CEST)
> "Felix Schumacher" <felix.schumacher at internetallee.de> wrote:
>
>> > Like I said the other day, the only way to fix this problem is to
>> create
>> > a custom lock class and replace all of the synchronized calls so that
>> one
>> > lock can be used and the lock can be *unlocked* deep in hte
>> call-stack.
>> So given an implementation of concurrent.ReentrantLock (with which one
>> could start to have a known working implementation), how would you
>> decide
>> when to unlock the transport and where?
>
> Hi Felix,
>
> If you have a lock class that can be unlocked then the solution is simple
> because you can have one lock for the entire VM. If there's only one
> lock then a deadlock is theoretically impossible.
If you only want to have ONE lock for the entire VM, it should be enough
to synchronize on a static Object Variable (which should exist only once
in the VM). But I had the impression, it would be nicer to lock every
instance of Transport seperatly. When we use only one Object to
synchronize there, it should not be possible to deadlock either. Or do we
only have one instance of Transport? In that case we effectivly have the
wanted situation already.
>
> Meaning, create a static instance of ReentrantLock associated with
> jcifs.util.transport.Transport and add static lock/unlock methods that
> lock/unlock that ReentrantLock. Now, eliminate all synchronized calls
> and replace them with Transport.lock() / Transport.unlock().
>
> That will solve the deadlock but there is a pitfall. You must never ever
> ever EVER block while that lock is locked. For example if the transport
> tries to read() from a socket with the lock locked the entire client
> will lockup (but not deadlock) waiting for that I/O. The end result will
> be that performance will go subterranean.
The performance will be bad having one lock only, since every write to all
Transport instances would be queued.

I thought you wanted to have a thread to disconnect old sessions, which
would lock the corresponding transport instance and wait for a condition
to be true (time, transport has too many sessions, ...). While this thread
waits it would release it's lock, so other threads could do their work.
When a working thread or a supervising thread signals that the awaited
condition is true, the sleeping cleanup thread would awake, reclaim it's
lock and do the dirty work, while keeping the other jobs at bay.

If the design of the cleanup job stays like it is today, I see no real win
in using a lock, but to loose the compiler checked synchronization.

Bye
 Felix
>
> If you actually do this and you know what you're doing and it works,
> please send a patch and I'll put it in the patches directory.
>
> Mike
>
> --
> Michael B Allen
> PHP Active Directory Kerberos SSO
> http://www.ioplex.com/
>




More information about the jcifs mailing list