Simon McNamara wrote:The code works well and is very tidy. But, of course, after a few weeks of this running 24/7, I encounter a stack overflow. Is there any formal way to guard against this?
Return when the count exceeds your desired limit.
Stephan van Hulst wrote:Please describe the problem you're trying to solve in more detail, and we might suggest some sample code.
Stephan van Hulst wrote:NEVER trigger execution of a task on another thread from a constructor. Add a start() method that you call AFTER your constructor has finished running. Otherwise, it is not guaranteed that the parallel task will observe the object to be in a fully initialized state, which can be a source of very insidious bugs.
Stephan van Hulst wrote:Why are you passing a TimerTask to an Executor? You can implement Runnable instead of extending TimerTask.
Stephan van Hulst wrote:Why are you using a low level concurrency primitive like Semaphore, instead of Java's high level concurrency classes, such as ReentrantLock? And why are you acquiring the lock only once but releasing it many times? And what critical section of your code is the lock trying to protect in the first place?
Stephan van Hulst wrote:It's questionable to call Object.wait() regardless of why you're doing it, but calling it on your executor service makes no sense and is probably not going to work the way you intended it to.
Campbell Ritchie wrote:You won't get your code to compile with Void as a return type because Void can't be instantiated. I think you mean void.
Simon McNamara wrote:I think I am acquring and releasing it many times. Each time mainPulseSequence is called I acquire the lock. It is only released after the routine inside of TimerTask finishes.
I tried using this.wait() on the Task Thread instead of a Semaphore object, but then nothing I tried could wake it again from the nested TimeTask thread.
Stephan van Hulst wrote:Before I can give you a good example, you need to explain carefully to me why the code inside the TimerTask needs to run asynchronously with respect to the code inside the Call() method, seeing as that code is already running asynchronously with respect to the front-end.
Don't let your services create their own executor services. Pass executors into your FilePrinter through the constructor. This applies to all services that your classes depend on, not just executors.
Again, for emphasis, NEVER trigger threads from constructors. In your previous code it was done indirectly. In your updated could you are literally calling Thread.start(). DON'T.
The root of the problem is that you're mixing TimerTask with ScheduledExecutorService. Just call ScheduledExecutorService.schedule() with a custom Runnable (NOT TimerTask) and let it run to its end. You don't need to cancel it if you run it only once.
Again, wait() does not do what you think it does. It does NOT suspend the task or executor that you call it on. Instead, it's similar to calling lock.acquire(), with the object that you're calling it on acting as a lock.
Simon McNamara wrote:That code does not strictly need to run asynchronously. It must run at a fixed delay.
I'm not clear why a service is any different from any other object. Why not just make them as needed and throw them away?
I suppose I can start the thread from the GUI itself, where the Task is created, but why not put it in the constructor. I can't see the risk. A class which extends Task isn't running concurrently until I call Task#call, so there's no risk of the constructor not finishing before other operations begin (as far as I know).
I will look into ScheduledExecutorService. From the cursory peek just now I see that it is syntactically similar to ScheduledThreadPoolExecutor. Looks like I pass the former a Task whereas I pass the latter a Runnable.
Even if I passed in services from the GUI thread, I would still need to pause them at times. How would I do this without #wait? Lock#acquire is pretty much exactly the behavior I want, so I don't see the conflict.
Careful: GUIs are usually not thread‑safe. I think you may be thinking you go from GUI to application, but you should usually go from application to GUI.
Simon McNamara wrote:. . . I suppose I can start the thread from the GUI itself
“Famous Last Words”
. . . there's no risk of the constructor not finishing before other operations begin (as far as I know). . . .