phrack / ShootOFF

A virtual shooting range to enhance laser dry fire training.
http://shootoffapp.com
GNU General Public License v3.0
133 stars 71 forks source link

Stackoverflow on close #685

Closed cbdmaul closed 7 years ago

cbdmaul commented 7 years ago

Exception in thread "JavaFX Application Thread" java.lang.StackOverflowError at java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:285) at java.util.concurrent.ThreadPoolExecutor.interruptWorkers(ThreadPoolExecutor.java:753) at java.util.concurrent.ThreadPoolExecutor.shutdownNow(ThreadPoolExecutor.java:1421) at java.util.concurrent.ScheduledThreadPoolExecutor.shutdownNow(ScheduledThreadPoolExecutor.java:786) at com.shootoff.gui.CanvasManager.close(CanvasManager.java:181) at com.shootoff.camera.CameraManager.close(CameraManager.java:281) at com.shootoff.camera.CameraManager.cameraClosed(CameraManager.java:827) at com.shootoff.camera.cameratypes.SarxosCaptureCamera.close(SarxosCaptureCamera.java:155) at com.shootoff.camera.cameratypes.CalculatedFPSCamera.setState(CalculatedFPSCamera.java:48) at com.shootoff.camera.CameraManager.setCameraState(CameraManager.java:316) at com.shootoff.camera.CameraManager.close(CameraManager.java:285) at com.shootoff.camera.CameraManager.cameraClosed(CameraManager.java:827) at com.shootoff.camera.cameratypes.SarxosCaptureCamera.close(SarxosCaptureCamera.java:155) at com.shootoff.camera.cameratypes.CalculatedFPSCamera.setState(CalculatedFPSCamera.java:48) at com.shootoff.camera.CameraManager.setCameraState(CameraManager.java:316) at com.shootoff.camera.CameraManager.close(CameraManager.java:285) at com.shootoff.camera.CameraManager.cameraClosed(CameraManager.ja

I don't understand why.

https://github.com/phrack/ShootOFF/blob/master/src/main/java/com/shootoff/camera/cameratypes/CalculatedFPSCamera.java#L46

Seems to indicate that it will NOT call close(); if it is already CLOSED. But it is still being called?

cbdmaul commented 7 years ago

This happens when leaving the pref window also

phrack commented 7 years ago

I am not duping this (should I be able to?). This is weird. The infinite loop I see in the stack trace does look like it should be prevented by the guard you linked to.

Having said that, I do see a related bug:

https://github.com/phrack/ShootOFF/blob/master/src/main/java/com/shootoff/camera/cameratypes/SarxosCaptureCamera.java#L155

Based on my understanding of the code, this line to call the listener should only happen in the conditional branch where closing is set to true. Otherwise, there is the potential it will be called twice for the same camera without an open in between. In otherwords it can notify me that an already closed camera is closing (whatever that means).

A similar case can be made for moving the openCameraRemove method in the same way.

cbdmaul commented 7 years ago

I made that change and it seemed to fix it. That code is ugly/illogical but I don't have the will to completely refactor it, so I am happy with a simple bug squash. Will reopen if I see it again (It happened every time for me though).