|
|
Boost : |
Subject: Re: [boost] gil::io "non-review" (was: [gil] Can not open test.jpg)
From: Phil Endecott (spam_from_boost_dev_at_[hidden])
Date: 2010-03-23 12:29:02
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/io_new/formats/jpeg/read.hpp
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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk