Locking in DeploymentRepository

[alesj]

While fixing JBAS-6330 I saw I need a lock on the HDScanner
as in 5_x we do check for modifications differently.
(multiple profiles, hence multiple urls)

I would need a lock on the HDScanner when Tools want to suspend it.
And then release that lock when they are ready to resume.

The issue is how to impl that, since Tools invocations will call from two diff threads,
and most Lock impls don't support that.
(btw: this should also be fixed in 5_0's SerializableDeploymentRepository).

How do I take a hold of HDScanner's monitor?

[starksm64]

Why doesn't just calling setScanEnabled suspend/resume the scanning?

[jason.greene]

It sounds like you need a simple flag, or alternatively a latch instead of a lock:
http://anonsvn.jboss.org/repos/jbosscache/core/trunk/src/main/java/org/jboss/cache/util/concurrent/ReclosableLatch.java

If you really want a lock that can be shared across threads, in order to conform to the Lock interface, you need to use a thread local. Manik and I created one specific to jboss cache transactional locks here (you can derive from it if you like):

http://anonsvn.jboss.org/repos/jbosscache/core/trunk/src/main/java/org/jboss/cache/util/concurrent/locks/OwnableReentrantLock.java

[alesj]

How does this fit with in-progress scan()?

As I think this will not cause scan to stop,
hence it might pick up partial deployments
since suspend would return immediately.

[starksm64]

"alesj" wrote:
How does this fit with in-progress scan()?

As I think this will not cause scan to stop,
hence it might pick up partial deployments
since suspend would return immediately.

It does cancel any inprogress scan, but how quickly that ends depends on the Future. Your looking to set the HDScanner in a known state of scanning disabled and any in progress scan completed/disabled?

[alesj]

Your looking to set the HDScanner in a known state of scanning disabled and any in progress scan completed/disabled?

Looking from Tools usage, I want to suspend HD thread to pick up
any partial deployments I might currently deploy.

Looking at HDScanner's activeScan I think I might get away with
calling cancel on it, and spinning a bit around until it's done (activeScan.isDone == true).

[alesj]

"alesj" wrote:

Looking at HDScanner's activeScan I think I might get away with
calling cancel on it, and spinning a bit around until it's done (activeScan.isDone == true).

 public synchronized void suspend()
 {
 suspended = (activeScan != null);
 if (activeScan != null)
 {
 activeScan.cancel(false);
 while(activeScan.isDone() == false)
 {
 try
 {
 Thread.sleep(100);
 }
 catch (InterruptedException ignored)
 {
 }
 }
 activeScan = null;
 }
 }

 public synchronized void resume()
 {
 if (suspended)
 {
 start();
 }
 suspended = false;
 }

Or too hack-ish? :-)

[starksm64]

adding suspend/resume ops that essentially mirror stop/start but wait until the state of the scanner has changed seems fine. The details of the impl can be hacky for now.

[alesj]

"alesj" wrote:

The issue is how to impl that, since Tools invocations will call from two diff threads,
and most Lock impls don't support that.
(btw: this should also be fixed in 5_0's SerializableDeploymentRepository).

How come this didn't pop-up before?

As PS tests should see this, when using DeploymentManager to do remote deployment.
Or is the thread that takes the stream, that does the suspend and resume?

I'll check this out once I properly port JBAS-6330 to 5_0.

[jason.greene]

"alesj" wrote:

Looking at HDScanner's activeScan I think I might get away with
calling cancel on it, and spinning a bit around until it's done (activeScan.isDone == true)

Or too hack-ish? :-)

Ugh. If you are waiting on a future, use get(), don't spin. Also, if a cancellation is successful, then isDone will immediately be true, even if the thread for the task is still running.

[alesj]

I think the future is an impl detail there.
Scott?

Also, if a cancellation is successful, then isDone will immediately be true, even if the thread for the task is still running.

How to avoid this?

[starksm64]

"alesj" wrote:
I think the future is an impl detail there.
Scott?

Its part of the HDScanner api in that it uses a ScheduledExecutorService via injection. If one is not provided it creates one.

[alesj]

The question is more "can I get a hold of that Future instance",
then, like Jason says, it would be just a matter of calling
Future::get to know when we're done with cancel.

[starksm64]

The ScheduledFuture is the activeScan variable. It could be exposed as a read-only property that would be null if scanning is not enabled.

[alesj]

Eh, I should read the code better. :-(
As you said, activeScan == Future. :-)