From 06bbb280c4646fb0e64866d55f78a95d5e173732 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Mon, 2 May 2016 02:26:15 -0400 Subject: java-segfault-redux: fix analysis of modern SmartDashboard --- public/java-segfault-redux.html | 4 +++- public/java-segfault-redux.md | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/public/java-segfault-redux.html b/public/java-segfault-redux.html index 55529c2..2f25f4d 100644 --- a/public/java-segfault-redux.html +++ b/public/java-segfault-redux.html @@ -75,7 +75,9 @@ public BufferedImage getBufferedImage() {

TheLoneWolfling on Hacker News noted that they'd be worried about the JVM optimizing out the call to getWidth(). I'm not, because WPIImage.getWidth() calls IplImage.width(), which is declared as native; the JVM must run it because it might have side effects. On the other hand, looking back, I think I just shrunk the window for things to go wrong: it may be possible for the garbage collection to trigger in the time between getWidth() being dispatched and width() running. Perhaps there was something in the C/C++ code that made it safe, I don't recall, and don't care quite enough to dig into OpenCV internals again. Or perhaps I'm mis-remembering the fix (which I don't actually have a file of), and I called some other method that could get optimized out (though I do believe that it was either getWidth() or getHeight()).

WPI's fix

Four years later, the SmartDashboard is still being used! But it no longer has this bug, and it's not using my workaround. So, how did the WPILib developers fix it?

-

Well, the code now lives in git at collab.net, so I decided to take a look. They put a mutex around all accesses to image, which should have been the obvious solution to me.

+

Well, the code now lives in git at collab.net, so I decided to take a look.

+

The stripped out WPIJavaCV from the main video feed widget, and now use a purely Java implementation of MPJPEG streaming.

+

However, the old video feed widget is still available as an extension (so that you can still do cool things with processImage), and it also no longer has this bug. Their fix was to put a mutex around all accesses to image, which should have been the obvious solution to me.