Closed GoogleCodeExporter closed 9 years ago
Upon investigation, there is a memory leak in 1.65 of the fp struct (568 bytes
in
linux) for each read and write.
This is due to not calling fclose(fp) for GIF reading and writing.
And that was put in a couple of years ago when we first started supporting GIF,
and were using an older, buggy version of giflib that called fclose(fp)
when the gif struct was destroyed. Yes, hard to believe.
Anyway, this leak goes away when the streams are closed.
To patch:
remove l. 164 in readfile.c
remove l. 243 in writefile.c
Let me know if that solves the halting problem on windows.
This will be fixed in 1.66.
Original comment by dan.bloo...@gmail.com
on 9 May 2010 at 11:38
Hi,
As i understand, i've removed the lines "164 in readfile.c" and "243 in
writefile.c
" respectively:
removed the line 164 in readfile.c which is like below by commenting the line:
if (pixGetInputFormat(pix) != IFF_GIF) /* DGifCloseFile() closes stream! */
removed the line 243 in writefile.c which is like below by commenting the line:
if (format != IFF_GIF) /* EGifCloseFile() closes file stream! */
Result is again a halting prpblem when i try to read gif files (writing is not
tested). Before, i could read and display the gif files, but problem occurs
when the
program is closed, and now, when i try to read the gif file, the halting problem
occurs immediately.
If i remove the correct lines from readfile.c and writefile.c, so what can be
done
more to solve the fault?
Thanks for your considerations.
Original comment by fatih...@gmail.com
on 10 May 2010 at 2:56
OK, you've removed the conditional lines, so that the stream is explicitly
closed
with fclose() for both reading and writing GIF files.
When you say "halting problem", do you mean that the program hangs (as if it
went
into an infinite loop), or that it crashes, or what?
What compiler are you using on windows?
Original comment by dan.bloo...@gmail.com
on 10 May 2010 at 6:36
Hi,
I've compiled Leptonica_v1-65 using VS2008 as a DLL file. If i use the modified
readfile.c/writefile.c then test program crashes when i try to read a gif file
immediately, so the program halts.
Before, without modifying two of these files, i could read and display, but the
program crashes when closing the program.
I think there is a problem with pixReadStream(). In the readfile.c:
...
case IFF_GIF:
if ((pix = pixReadStreamGif(fp)) == NULL)
...
there can be a problem in pixReadStreamGif(fp).
Thanks.
Original comment by fatih...@gmail.com
on 10 May 2010 at 6:56
There is a specific section in the VS2008 Notes on adding giflib 4.1.6 support
to
Leptonica at http://www.leptonica.org/vs2008doc/building-giflib.html. I've only
tested it on Windows XP Pro, however, I did verify that the gifio_reg program
runs
correctly (that's how I got the screenshot at the bottom of the page).
Can you build & run gifio_reg without errors?
Original comment by tomp2...@gmail.com
on 10 May 2010 at 7:06
Hi,
I guess that the gif file (GifFileType * GifFile struct) must be closed when
reading
using DGifCloseFile( GifFileType * GifFile ) and using EGifCloseFile(
GifFileType *
GifFile ) if writing.
Original comment by fatih...@gmail.com
on 10 May 2010 at 7:09
Looks to me like DGifCloseFile() in dgif_lib.c at line 610 still does:
if (File && (fclose(File) != 0)) {
_GifError = D_GIF_ERR_CLOSE_FAILED;
return GIF_ERROR;
}
And EGifCloseFile() in egif_lib.c at line 760 also does:
if (File && fclose(File) != 0) {
_GifError = E_GIF_ERR_CLOSE_FAILED;
return GIF_ERROR;
}
So readfile.c & writefile.c were correct in 1.65. Changing them as suggested in
Comment 1 would result in the same errors resolved in Issue 28
(http://code.google.com/p/leptonica/issues/detail?id=28&can=1)
Original comment by tomp2...@gmail.com
on 10 May 2010 at 7:27
Maybe this is some difference between giflib 4.1.6 and libungif 4.1.4 (which
are NOT
the same thing). giflib 4.1.6 is newer so that's what I used.
Original comment by tomp2...@gmail.com
on 10 May 2010 at 7:44
Yes, it could be a 4.1.6 vs ungiflib 4.1.4 issue.
But we have this situation on 4.1.6:
(a) with the patch of 28:
linux: memory leak of about 600 bytes (the stream) on each read and write.
windows: perhaps it leaks here as well?
(b) without the patch of 28:
linux: no memory leak and no memory corruption (e.g., there is no double-free
of the stream!)
windows: crashes
giflib seems to be stashing a "private" version of the ptr to fp, and using
it to close the stream, as you pointed out in dgif_lib.c.
This is baffling. Should we close the stream in linux but not in windows?
Original comment by dan.bloo...@gmail.com
on 10 May 2010 at 11:41
I wrote a little testing program:
FILE *fp = fopen("marge.gif", "rb");
l_int32 fd = fileno(fp);
fprintf(stderr, "fd = %d\n", fd);
FILE *fp2 = fdopen(fd, "rb");
fprintf(stderr, "fp = %x, fp2 = %x\n", fp, fp2);
fclose(fp2);
fclose(fp);
As expected, fdopen makes a new stream object. That's why it
is necessary in linux to fclose(fp) -- because the stream that
is closed by DGifCloseFile() is a different stream, private to the 'GifFile'.
Each stream that is not closed causes a memory leak of 568 bytes.
The only difference in windows is line 120 in dgif_lib.c, where
setvbuf is called to associate a new buffer with the stream.
But I don't understand what's really happening there.
Original comment by dan.bloo...@gmail.com
on 11 May 2010 at 12:10
The VS2008 documentation on setvbuf() is at
http://msdn.microsoft.com/en-us/library/86cebhfs%28VS.90%29.aspx.
Seems harmless enough with the only question being what happens to the
"automatically
allocated" buffer. Presumably it is "automatically" freed when the stream is
closed.
I would still like to hear if the OP built and ran gifio_reg without errors.
Like I
said, it works fine for me.
Original comment by tomp2...@gmail.com
on 11 May 2010 at 5:56
To remove the linux error leak, we can put this code in readfile.c (and similar
in writefile.c):
/* Close the stream except if GIF under windows, because
* DGifCloseFile() closes the windows file stream! */
if (pixGetInputFormat(pix) != IFF_GIF)
fclose(fp);
#ifndef COMPILER_MSVC
else /* gif file */
fclose(fp);
#endif /* !COMPILER_MSVC */
w/ issue 36: yes, it would be very useful to be able to find
memory leaks in windows, esp. in cases such as this where the
low-level calls differ from linux.
Original comment by dan.bloo...@gmail.com
on 11 May 2010 at 4:14
the changes described above were just released with 1.66.
Does this solve all the crash problems in windows, so we can close this?
Original comment by dan.bloo...@gmail.com
on 11 Aug 2010 at 7:42
I was able to run gifio_reg without any problems so I would say yes. If the
original poster still finds a problem he'll need to re-open the issue.
Original comment by tomp2...@gmail.com
on 11 Aug 2010 at 8:08
Original comment by dan.bloo...@gmail.com
on 11 Aug 2010 at 9:55
Hi,
Leptonlib v1.66 is very good and gif problem is fixed, i've tested (OS: Vista
Home Premium, the library is compiled in VStudio2008 IDE, tested in a simple
VC++ program and second test done using a delphi executable).
Good and fast.
Thanks for All.
Fatih.
Original comment by fatih...@gmail.com
on 11 Aug 2010 at 11:27
Hi, I need delphi wrapper for Leptonica! Can You send me please this wrapper?
And you'r vs2008 project for leptonica?
Mail to: andruxa.smirnov@gmail.com
Best regards, Andrew Smirnov
Original comment by andruxa....@gmail.com
on 15 Aug 2010 at 4:52
I don't use Delphi myself but I believe the best approach would be to download
the Leptonica pre-built binaries for Windows (32-bit) from
http://www.leptonica.org/source/leptonica-1.66-win32-lib-include-dirs.zip and
use leptonlib.dll from Delphi.
See "Using C DLLs with Delphi" (http://www.drbob42.com/delphi/headconv.htm) for
details on how you do this. There is also a tool called HeadConv aka "Project
JEDI Header Converter utility" that you can get from
http://www.delphi-jedi.org/api-howto.html that will apparently automatically
create the Delphi wrapper stuff you need.
Download the vs2008 project from
http://www.leptonica.org/source/vs2008-1.66.zip (although that's mainly if you
need to rebuild the leptonica.dll for some reason).
Original comment by tomp2...@gmail.com
on 15 Aug 2010 at 6:19
Original issue reported on code.google.com by
fatih...@gmail.com
on 9 May 2010 at 2:48