Christian Henning wrote:
Phil Endecott wrote:
In jpeg/read.hpp you have a namespace-scope static jmp_buf. ?I immediately worry about thread-safety. ?Even in a single-threaded app I can't see how this works if you have more than one jpeg file open. ?Then you setjmp() in the constructor. ?The jmp_buf is invalid as soon as that scope returns, so if libjpeg calls your error_exit() from after the ctor has finished (e.g. in jpeg_read_scanlines()) it will crash. ?(Have I grossly misunderstood how this works?)
Let's forget multi-thread safety for a bit. During the construction of the my jpeg_decompress_mgr I tell libjpeg's jpeg_error_mgr to use my own error handler ( static member function called error_exit() ). After this, I save the current environment into the marker ( int[16] buffer ) using the setjmp() function. The marker is a static global inside the gil::detail namespace. Once an error occurs error_exit() is called which leads to a longjmp which then jumps back to the setjmp point.
It only jumps back to the setjmp point if the stack context in which setjmp was called is still valid. setjmp is called from the jpeg_decompress_mgr constructor. You make libjpeg calls (e.g. jpeg_read_scanlines()) after the constructor has finished, right? This will cause undefined behaviour if errors occur in those calls when it invokes longjmp. (Have you tested this?)
Next we clean up and issue an exception. Hope this makes sense.
No, it doesn't make sense. Sorry if I am missing something important. Maybe someone else would like to have a look at this code? It's here: http://gil-contributions.googlecode.com/svn/trunk/gil_2/boost/gil/extension/... near the start of the file.
In a single-treaded application how would this procedure be a problem? I mean how can you have more than one jpeg file open?
Errr... you cannot have more than one jpeg file open? I suppose not. I guess that illustrates the limitations of this approach.
For multi-threaded usage the above is problematic since the marker can be overridden. In case the user reads several jpeg files at one time this can lead to undefined behavior. One solution would be to create a marker for each read operation. Do you thing that's a valid approach?
No; it seems to me that the whole idea of using setjmp/longjmp is broken. But as I say I have only spent a few minutes looking at this code. I would be very grateful if someone else could take a look at it. Regards, Phil.
