Re: Syste.OutOfMemoryException - C# Discussion Boards

11-Mar-09 14:34

Hi Jon,

adding some GC.Collect() calls is not the way to fix programming mistakes. What you need to do is call Dispose() on those objects you don't need any longer, provided their class has such a method.

In your case, insertLib = new OleDbCommand(insSql, musLibConn); is the most likely culprit: inside the loop you create OleDbCommand instances, use them, then let them fade into oblivia (by assigning a new OleDbCommand to insertLib, the old one is gone, but still occupies memory.

I suggest you do not declare OleDbCommand insertLib; outside the foreach loop, instead do

foreach (...) {
  OleDbCommand insertLib=new OleDbCommand(...);
  ...
  insertLib.Dispose();
}

that makes it perfectly clear the OleDbCommand is always created, used, disposed.

I am not familiar with some of the classes you are using (e.g. ID3Info), chances are they also have a Dispose() and need the same treatment.

ADDED
There is a useful construct that automates the above, like so:

foreach (...) {
  using (OleDbCommand insertLib=new OleDbCommand(...)) {
     ...
  }
}

The advantages are it takes less code, and the Dispose is guaranteed to occur, even when an exception gets thrown inside the using block.
/ADDED

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets

modified on Wednesday, March 11, 2009 8:43 PM

DaveyM6911-Mar-09 14:40

11-Mar-09 14:40

I assume that the ID3info class (not shown) opens the file from the supplied parameters.

CurrSong = new ID3Info(filepaths[i], true);

If you have access to this classes code, make sure that the stream is disposed, if not, check to see if it has a Close or Dispose method. That might mean moving the
ID3Info CurrSong; inside the foreach loop.

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

DaveyM6911-Mar-09 14:45

11-Mar-09 14:45

On a minor note... you have

foreach(string str in filepaths)

then

path = filepaths[i].ToString();

The path field is unecessary, you can just use str in it's place.

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

Member 461797111-Mar-09 14:48

11-Mar-09 14:48

I have brought the declarations inside the loop, removed the System.GC.Collect and put in the insertLib.dispose(). The ID3Info class has no dispose or close method. Running the code now doesnt get me as far as with System.GC.Collect.

DaveyM6911-Mar-09 14:52

11-Mar-09 14:52

Check out Luc's added note to his previous post about wraping in a using block.

That might be the answer to the ID3Info as well (if that's the culprit). You can have nested using blocks Smile | :)

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

Member 461797111-Mar-09 14:59

11-Mar-09 14:59

Works for the OleDbCommand but not the ID3Info

It give s me this

'ID3.ID3Info': type used in a using statement must be implicitly convertible to 'System.IDisposable'

DaveyM6911-Mar-09 15:26

11-Mar-09 15:26

I've had a quick look at that code, each time you call ID3Info(string FilePath, bool LoadData), it calls ID3v1(string FilePath, bool LoadData) which calls Load() in the same class (if LoadData is true) which has FS.Close(); as the penultimate line, so that shouldn't be the source of your problem.

You could try changing the ID3v1 Load method to:

using (FileStreamEx FS = new FileStreamEx(_FilePath, FileMode.Open))
{
    _HaveTag = false;
    if (FS.HaveID3v1())
    {
        _Title = FS.ReadText(30, TextEncodings.Ascii);
        FS.Seek(-95, SeekOrigin.End);
        _Artist = FS.ReadText(30, TextEncodings.Ascii);
        FS.Seek(-65, SeekOrigin.End);
        _Album = FS.ReadText(30, TextEncodings.Ascii);
        FS.Seek(-35, SeekOrigin.End);
        _Year = FS.ReadText(4, TextEncodings.Ascii);
        FS.Seek(-31, SeekOrigin.End);
        _Comment = FS.ReadText(28, TextEncodings.Ascii);
        FS.Seek(-2, SeekOrigin.End);
        _TrackNumber = FS.ReadByte();
        _Genre = FS.ReadByte();
        _HaveTag = true;
    }
}

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

Luc Pattyn11-Mar-09 15:46

11-Mar-09 15:46

DaveyM69 wrote:
so that shouldn't be the source of your problem.

unless an exception gets thrown, and got swallowed and/or not reported in the post.
Anyway your using construct fixes that.
The file data itself is the only large thing I could spot, nothing else could exhaust memory with less than 25 files, except just maybe the very first Directory.GetAllFiles().

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

DaveyM6911-Mar-09 15:56

11-Mar-09 15:56

Luc Pattyn wrote:
except just maybe the very first Directory.GetAllFiles().

There would have to be a LOT of files for that to happen, but it may be possible.

Luc Pattyn wrote:
unless an exception gets thrown

I didn't look at the FilStreamEx class, but I didn't see any try/catch or usings in the ID3Info or ID3v1 classes I looked at, so it's a possibility if they're not caught elsewhere.

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

Luc Pattyn11-Mar-09 16:06

11-Mar-09 16:06

as far as that article goes there isn't a single "try" keyword in the entire solution.

there are several static lists/collections but I doubt they could really grow large with only 25 files.

I think I got it: there is a MemoryStream "MStream" inside FileStreamEx, it never gets Disposed().

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

DaveyM6911-Mar-09 23:03

11-Mar-09 23:03

Good find. I got too tired to be looking over countless classes that weren't written by the OP and went to bed!

Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)

Member 461797111-Mar-09 15:06

11-Mar-09 15:06

I am using this class

http://www.codeproject.com/KB/cs/Do_Anything_With_ID3.aspx?fid=393894&fr=76#xx0xx[^]

Luc Pattyn11-Mar-09 15:13

11-Mar-09 15:13

Please show the code again, the way it looks now.

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

Member 461797111-Mar-09 15:16

Re: Syste.OutOfMemoryException [modified]

11-Mar-09 15:16

int i = 0,duration, year;
                string artist, title, album, bandname, genre, path;
                string delSql = "DELETE * FROM Library";
                musLibConn.ConnectionString = @"Provider=Microsoft.Jet.OLEDB.4.0;Data Source=C:\Program Files\GeneSys\Library.mdb";
                OleDbCommand deleteLib = new OleDbCommand(delSql, musLibConn);
                musLibConn.Open(); 
                if (tbMusPath.Text != "")
                {
                    utility util = new utility();
                    filepaths = Directory.GetFiles(util.getLibPath(), "*.mp3", SearchOption.AllDirectories);
                    deleteLib.ExecuteNonQuery();
                    pBar.Visible = true;

                    foreach(string str in filepaths)
                    {
                        //pBar.Value = i % 100;
                        //path = filepaths[i].ToString();
                        //duration = Convert.ToInt32(CurrSong.ID3v2Info.GetTextFrame("TLEN"));
                        lblPath.Text = str;

                        ID3Info CurrSong = new ID3Info(filepaths[i], true);
                        artist = CurrSong.ID3v2Info.GetTextFrame("TPE1");
                        title = CurrSong.ID3v2Info.GetTextFrame("TIT2");
                        album = CurrSong.ID3v2Info.GetTextFrame("TALB");
                        
                        string insSql = "INSERT INTO Library(Artist,Title,Album,Path)" +
                                        "VALUES('" + artist.Replace("'", "''") + "','" + title.Replace("'", "''") + "','" +
                                        album.Replace("'", "''") + "','" + str.Replace("'", "''") + "')"; 
                        using (OleDbCommand insertLib = new OleDbCommand(insSql, musLibConn))
                        {                           
                            insertLib.ExecuteNonQuery();
                        }
                        i++;
                    }
                    
                    pBar.Visible = false;
                    lblNum.Text = Convert.ToString(filepaths.Length);
                    util.setNoFiles(filepaths.Length);

Luc Pattyn11-Mar-09 15:49

11-Mar-09 15:49

Thanks.
Could you tell us approximately how many files are there, and what would be their typical size.

Are there any exceptions thrown while the app runs (I mean before the OutOfMemory of course)?

[ADDED]
Did you check for any errors in the ID3Info class? There is a Clear() method, and Count and List properties that would return the number and the array or errors.
[/ADDED]

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

Member 461797111-Mar-09 16:53

11-Mar-09 16:53

sorry for taking so long.

RIght now there are approximately 2000 files, with the potential for 40,000 at between 3 and 6 mb.

The program runs fine up until I build the database and get the memory exception.

I will try your other suggestion about the filestream class.

Luc Pattyn11-Mar-09 16:10

11-Mar-09 16:10

Hi,

the FileStreamEx class has a ReadText method that creates a MemoryStream but never disposes of it.
That is bad. I suggest you replace line 90

return GetEncoding(TEncoding).GetString(MStream.ToArray());

string result=GetEncoding(TEncoding).GetString(MStream.ToArray());
MStream.Dispose();
return result;

It is likely that will dramatically improve the situation.

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

Member 461797111-Mar-09 16:55

11-Mar-09 16:55

doing this throws the same exception but from the binaryFrame.cs

Luc Pattyn11-Mar-09 17:13

11-Mar-09 17:13

You have the failing app in front of you, no one else has, so you should try and provide all available information, such as:
1. is it better, i.e. are more files being handled than before, how many?
2. everything you have about the exception, including the stack traceback including line numbers.

Anyway, I'm calling it a day now.

Smile | :)

Luc Pattyn [Forum Guidelines] [My Articles]

Member 461797111-Mar-09 17:48

11-Mar-09 17:48

Sorry for the vagueness

It is handling less than before, doesnt appear to be handling more than 2 or three now.

Same exception, different place. BinaryFrame.cs Line 31

28 internal BinaryFrame(string FrameID, FrameFlags Flags, FileStreamEx Data, int Length)
29 : base(FrameID, Flags)
30 {
31 _Data = Data.ReadData(Length);
32 }

StackTrace

at System.IO.FileStreamEx.ReadData(Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\FileStreamEx.cs:line 147
at ID3.ID3v2Frames.BinaryFrames.BinaryFrame..ctor(String FrameID, FrameFlags Flags, FileStreamEx Data, Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\Frames Classes\BinaryFrame.cs:line 31
at ID3.ID3v2.AddFrame(FileStreamEx Data, String FrameID, Int32 Length, FrameFlags Flags) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 1120
at ID3.ID3v2.ReadFrames(FileStreamEx Data, Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 896
at ID3.ID3v2.Load() in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 443
at ID3.ID3v2..ctor(String FilePath, Boolean LoadData) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 77
at ID3.ID3Info..ctor(String FilePath, Boolean LoadData) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3Info.cs:line 25
at JukeBox.frmOptions.btnBLib_Click(Object sender, EventArgs e) in C:\Users\Jon Henry\Documents\Visual Studio 2005\Projects\JukeBox\JukeBox\frmOptions.cs:line 116
at System.Windows.Forms.Control.OnClick(EventArgs e)

Setting Form2's start position as Form1's last position

Ennis Ray Lynch, Jr.11-Mar-09 19:46

Ennis Ray Lynch, Jr.

11-Mar-09 19:46

First, don't mix case in the same scope. Local variables are usually either camelCase, or PascalCase but doing both is very poor practice, most people use camelCase. Watch the types of your objects, calling ToStrings on strings shows a poor thought process. Also, don't update the GUI in your loop (lblPath.Text = path) it can be part of the problem and most likely will not update with any regularity.

I do not know if this will work but you can try the code below: (Fix any of my typing errors as I typed it in this window), I don't know if JET supports prepared parameters but if it does you can call command.Prepare() before the loop begins.

OleDbCommand command = new OleDbCommand();
command.Connection = connection;
command.CommandText = "INSERT INTO library(artist, title, album, path) VALUES (@artist, @title, @album, @path);"
command.Parameters.Add("@artist", OleDbType.VarChar);
command.Parameters.Add("@title", OleDbType.VarChar);
command.Parameters.Add("@album", OleDbType.VarChar);
command.Parameters.Add("@path", OleDbType.VarChar);
string sqlString = ";
foreach(string path in filePaths){
    currentSong = new ID3Info(path, true);  //Is this disposable? If so use using(currentSong = new ID3Info(path, true){
    command.Parameters["@artist"].Value = currentSong.ID3v2Info.GetTextFrame("TPE1");
    command.Parameters["@title"].Value = currentSong.ID3v2Info.GetTextFrame("TIT2");
    command.Parameters["@album"].Value = currentSong.ID3v2Info.GetTextFrame("TALB");
    command.Parameters["@path"].Value = path;
    command.ExecuteNonQuery();
}

Need custom software developed? I do C# development and consulting all over the United States.

A man said to the universe:
"Sir I exist!"
"However," replied the universe,
"The fact has not created in me
A sense of obligation."
--Stephen Crane

Paulo Toureiro11-Mar-09 13:05

Paulo Toureiro

11-Mar-09 13:05

Hey folks,

I am very new to c# and have a small problem.

The problem :

My mainForm loads up in the CenterScreen position, if I open a new form(form2) it currently opens in CenterScreen position as well.
When form2 loads, form1 falls away.

What I'd like to achieve :

If I moved my mainForm before calling form2 up, I would like form2 to appear in the mainForm's last position.

Thanks in advance for any guidance in this matter.

Paulo.

Re: Setting Form2's start position as Form1's last position

DaveyM6911-Mar-09 13:21