473,320 Members | 1,814 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,320 software developers and data experts.

Possible Threading Issue

I have this class that provides a disk-based queue for storing various
items. As items are queued the event delegate is called and the consumer
dequeues the object and processes it. This seems to work off and on.
Sometimes the queue does not pick up a queued item until another item is
queued. Can anyone see a problem with this class?

***
Here's the interface and EventArgs...
***
/// <summary>
/// Event called when items are queued.
/// </summary>
public delegate void ItemQueuedEventHandler(object sender,
ItemQueuedEventArgs e);
/// <summary>
/// Item queued event data.
/// </summary>
public class ItemQueuedEventArgs : System.EventArgs
{
internal ItemQueuedEventArgs(int count) : base()
{
this._count = count;
}
/// <summary>
/// Count of items in the queue.
/// </summary>
public int Count
{
get { return this._count; }
} int _count = 0;
}
/// <summary>
/// Base definition of queue types.
/// </summary>
public interface IQueue : IDisposable
{
/// <summary>
/// Current count of all items in the queue.
/// </summary>
int Count { get; }
/// <summary>
/// Removes all objects in the queue.
/// </summary>
void Clear();
/// <summary>
/// Adds an object to the queue.
/// </summary>
/// <param name="obj">The object to add.</param>
void Enqueue(object obj);
/// <summary>
/// Removed an item from the queue.
/// </summary>
/// <returns>The first object from the top of the queue.</returns>
object Dequeue();
/// <summary>
/// Event called when items are queued.
/// </summary>
event ItemQueuedEventHandler ItemQueued;
}

***
Here's the class...
***
using System;
/// <summary>
/// Provides an event enabled disk-based queue.
/// </summary>
public class DiskQueue : IQueue
{
/// <summary>
/// Event called when new items are queued.
/// </summary>
public event ItemQueuedEventHandler ItemQueued;
/// <summary>
/// Extension appended to all files in the queue.
/// </summary>
public readonly string Extension = ".q";
System.IO.FileSystemWatcher _fsw = null;
static readonly object diskQueueLock = new object();
/// <summary>
/// Creates a new instance of the DiskQueue class.
/// </summary>
/// <remarks>
/// All items queued will be serialized with a binary formatter.
/// You can retrieve the queue location from the Path property.
/// </remarks>
public DiskQueue()
{
this.Path = this._path;
}
/// <summary>
/// Creates a new instance of the DiskQueue class.
/// </summary>
/// <param name="path">The path to place queued items.</param>
/// <remarks>All items queued will be serialized with a binary
formatter.</remarks>
public DiskQueue(string path)
{
this.Path = path;
}
/// <summary>
/// Creates a new instance of the DiskQueue class.
/// </summary>
/// <param name="formatter">The formatter to use for serialization.</param>
/// <param name="path">The path to place queued items.</param>
public DiskQueue(string path, System.Runtime.Serialization.IFormatter
formatter)
{
this.Formatter = formatter;
this.Path = path;
}
/// <summary>
/// Gets an object to synchronize access to the collection.
/// </summary>
public object SyncRoot
{
get { return diskQueueLock; }
}
/// <summary>
/// Gets or sets the formatter used to serialize queued items.
/// </summary>
/// <exception cref="System.ArgumentNullException">Formatter is
null.</exception>
/// <remarks>Be careful changing the formatter after instantiation. If there
are items
/// left in the queue that were formatted with the former they will throw
exceptions
/// on Dequeue.</remarks>
public System.Runtime.Serialization.IFormatter Formatter
{
get { return this._formatter; }
set
{
if (value == null)
throw new ArgumentNullException("Formatter");
this._formatter = value;
}
} System.Runtime.Serialization.IFormatter _formatter = new
System.Runtime.Serialization.Formatters.Binary.Bin aryFormatter();
/// <summary>
/// Gets or sets the path to the queue folder.
/// </summary>
/// <exception cref="System.ArgumentNullException">Path is null.</exception>
public string Path
{
get { return this._path; }
set
{
if (value == null)
throw new ArgumentNullException("Path");
string path = value;
if (path.EndsWith("\\")) path = path.Substring(0, path.Length - 1);
this._path = path;
if (!System.IO.Directory.Exists(this.Path))
System.IO.Directory.CreateDirectory(this.Path);
if (this._fsw != null)
this._fsw.Path = path;
}
} string _path =
System.Environment.GetFolderPath(System.Environmen t.SpecialFolder.CommonAppl
icationData) + Guid.NewGuid().ToString();
/// <summary>
/// Get the number of items in the queue.
/// </summary>
public int Count
{
get
{
return System.IO.Directory.GetFiles(this.Path, "*" + this.Extension).Length;
}
}
/// <summary>
/// Begins watching the folder for queued items and raises an event when
items are queued.
/// </summary>
public void Start()
{
this._fsw = new System.IO.FileSystemWatcher(this.Path, "*" +
this.Extension);
this._fsw.EnableRaisingEvents = true;
this._fsw.Created += new System.IO.FileSystemEventHandler(_fsw_Created);
this._fsw.Changed += new System.IO.FileSystemEventHandler(_fsw_Created);
this._fsw.Renamed += new System.IO.RenamedEventHandler(_fsw_Renamed);
}
/// <summary>
/// Adds an item to the queue.
/// </summary>
/// <param name="graph">The object to queue.</param>
/// <exception cref="System.Exception">Could not enqueue the
item.</exception>
/// <exception
cref="System.Runtime.Serialization.SerializationEx ception">Error serializing
the item.</exception>
/// <exception cref="System.ArgumentNullException"><c>graph</c> was
null.</exception>
public void Enqueue(object graph)
{
if (graph == null)
throw new ArgumentNullException("graph");
Exception e = null;
System.Threading.Thread.Sleep(10);
lock (diskQueueLock)
{
try
{
string file = this.Path + "\\" + DateTime.Now.Ticks.ToString();
System.IO.FileStream fs = System.IO.File.Create(file);
this.Formatter.Serialize(fs, graph);
fs.Close();
System.IO.File.Move(file, file + this.Extension);
}
catch (System.Runtime.Serialization.SerializationExcepti on sex)
{
e = sex;
}
catch (Exception ex)
{
e = ex;
}
}
if (e != null)
throw e;
}
/// <summary>
/// Removes an item from the queue.
/// </summary>
/// <returns>The deserialized object.</returns>
/// <exception cref="System.Exception">Could not dequeue the
item.</exception>
/// <exception
cref="System.Runtime.Serialization.SerializationEx ception">Error
deserializing the item.</exception>
/// <exception cref="System.NullReferenceException">No items
queued.</exception>
public object Dequeue()
{
if (this.Count < 1)
throw new NullReferenceException("No items queued!");
Exception e = null;
object o = null;
lock (diskQueueLock)
{
try
{
string file = System.IO.Directory.GetFiles(this.Path, "*" +
this.Extension)[0];
System.IO.FileStream fs = System.IO.File.Open(file, System.IO.FileMode.Open,
System.IO.FileAccess.Read, System.IO.FileShare.Read);
o = this.Formatter.Deserialize(fs);
fs.Close();
System.IO.File.Delete(file);
}
catch (System.Runtime.Serialization.SerializationExcepti on sex)
{
e = sex;
}
catch (Exception ex)
{
e = ex;
}
}
if (e != null)
throw e;
else
return o;
}
/// <summary>
/// Clears all items in the queue.
/// </summary>
/// <exception cref="System.Exception">Error clearing the queue.</exception>
public void Clear()
{
Exception iex = null;
Exception e = null;
lock (diskQueueLock)
{
try
{
foreach (string file in System.IO.Directory.GetFiles(this.Path, "*" +
this.Extension))
{
try { System.IO.File.Delete(file); }
catch (Exception ex) { iex = ex; }
}
if (iex != null)
e = iex;
}
catch (Exception ex)
{
e = ex;
}
}
if (e != null)
throw e;
}
private void _fsw_Created(object sender, System.IO.FileSystemEventArgs e)
{
if (this.ItemQueued != null)
this.ItemQueued(typeof(DiskQueue).ToString(), new
ItemQueuedEventArgs(this.Count));
}
private void _fsw_Renamed(object sender, System.IO.RenamedEventArgs e)
{
this._fsw_Created(sender, e);
}
#region IDisposable Members
/// <summary>
/// Destroys this instance and releases all resources.
/// </summary>
public void Dispose()
{
try
{
this._formatter = null;
//this.Clear();
if (this._fsw != null)
{
this._fsw.Created -= new System.IO.FileSystemEventHandler(_fsw_Created);
this._fsw.Renamed -= new System.IO.RenamedEventHandler(_fsw_Renamed);
this._fsw.Dispose();
}
}
catch { /* ignored */ }
}
#endregion
}
}
Nov 15 '05 #1
2 1704
I haven't investigated it to the point where I know why you observe that
behavior but there are a few questionable things in the code that I'll point
out in the code in method Enqueue (and similarly for the Dequeue method).

1. What is the purpose of the sleep statement? What happens if you remove
it, and what problem are you trying to solve with it?
My experience is that this usually is masking some other problem.

2. You should close the FileStream object in a finally block. The way the
code is structured is such that if the binary serializer ever throws an
exception the fs object's close method will never get called. This will leak
resources and perhaps cause other problems.

You can instead use (error handling omitted)...

try
{
using ( System.IO.FileStream fs = System.IO.File.Create(file) )
{
this.Formatter.Serialize(fs, graph);
}
}
catch <snip>

3. Minor point: I'd change this statement....
string file = this.Path + "\\" + DateTime.Now.Ticks.ToString();
A more platform neutral means of building a path is to use
string file =
System.IO.Path.Combine(this.Path,DateTime.Now.Tick s.ToString()); // uses
correct separator
Also, I'd probably test for a duplicate file name and do something like
this...
while ( System.IO.File.Exists(file) )
file += "_"; // this is probably more paraniod then you need to be.

4. You catch and save the exception object in each of the catch blocks but
all you do with it is rethrow the same object. This will actually cause a
loss of debugging information. A catch handler further up the call chain
that examines the stack trace will see the line of code where you caught the
exception as being the ultimate source rather then the actual line of code
that caused the exception. If all you do is catch and rethrow the same
object without adding additional context information there is no benefit to
the catch (unless you are deliberately trying to hide information).

If you want to add extra context information a better strategy is to do
something like this...

try { ... code here }
catch(System.Runtime.Serialization.SerializationEx ception sex)
{
throw new ApplicationException("Unable to serialize object; is object
valid?",sex);
}
catch (Exception ex)
{

throw new ApplicationException("Unknown error enqueueing item.",ex);
}

This preserves the original exception and allows you to add some context to
it. In general if you don't do anything useful in a catch block you are
better off not catching it.

"Nicholas" <nr******@nospam-hotmail-nospam.com> wrote in message
news:ez**************@TK2MSFTNGP12.phx.gbl...
I have this class that provides a disk-based queue for storing various
items. As items are queued the event delegate is called and the consumer
dequeues the object and processes it. This seems to work off and on.
Sometimes the queue does not pick up a queued item until another item is
queued. Can anyone see a problem with this class?

<snip>
Nov 15 '05 #2
Thanks for the response. I've updated my code using your suggestions.

Since I am basing the file names on time, I used the sleep to wait before
creating a new file. Your idea with the loop is better.

I overlooked the placement of the fs.Close() call and changed that as well.

I'm going to test again and see if I still experience the seemingly random
"issue". Any other suggestions are welcome.

Nick
"Dave" <no****************@wi.rr.com> wrote in message
news:ew**************@TK2MSFTNGP09.phx.gbl...
I haven't investigated it to the point where I know why you observe that
behavior but there are a few questionable things in the code that I'll point out in the code in method Enqueue (and similarly for the Dequeue method).

1. What is the purpose of the sleep statement? What happens if you remove
it, and what problem are you trying to solve with it?
My experience is that this usually is masking some other problem.

2. You should close the FileStream object in a finally block. The way the
code is structured is such that if the binary serializer ever throws an
exception the fs object's close method will never get called. This will leak resources and perhaps cause other problems.

You can instead use (error handling omitted)...

try
{
using ( System.IO.FileStream fs = System.IO.File.Create(file) )
{
this.Formatter.Serialize(fs, graph);
}
}
catch <snip>

3. Minor point: I'd change this statement....
string file = this.Path + "\\" + DateTime.Now.Ticks.ToString();
A more platform neutral means of building a path is to use
string file =
System.IO.Path.Combine(this.Path,DateTime.Now.Tick s.ToString()); // uses
correct separator
Also, I'd probably test for a duplicate file name and do something like
this...
while ( System.IO.File.Exists(file) )
file += "_"; // this is probably more paraniod then you need to be.

4. You catch and save the exception object in each of the catch blocks but
all you do with it is rethrow the same object. This will actually cause a
loss of debugging information. A catch handler further up the call chain
that examines the stack trace will see the line of code where you caught the exception as being the ultimate source rather then the actual line of code
that caused the exception. If all you do is catch and rethrow the same
object without adding additional context information there is no benefit to the catch (unless you are deliberately trying to hide information).

If you want to add extra context information a better strategy is to do
something like this...

try { ... code here }
catch(System.Runtime.Serialization.SerializationEx ception sex)
{
throw new ApplicationException("Unable to serialize object; is object
valid?",sex);
}
catch (Exception ex)
{

throw new ApplicationException("Unknown error enqueueing item.",ex);
}

This preserves the original exception and allows you to add some context to it. In general if you don't do anything useful in a catch block you are
better off not catching it.

"Nicholas" <nr******@nospam-hotmail-nospam.com> wrote in message
news:ez**************@TK2MSFTNGP12.phx.gbl...
I have this class that provides a disk-based queue for storing various
items. As items are queued the event delegate is called and the consumer
dequeues the object and processes it. This seems to work off and on.
Sometimes the queue does not pick up a queued item until another item is
queued. Can anyone see a problem with this class?

<snip>

Nov 15 '05 #3

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

0
by: Zuel | last post by:
Sup everyone! I wrote this code for Tomcat appserver but I am told from an associate that it has threading issues. The basic idea is to store a read only data table for everyone to use. It...
19
by: Jane Austine | last post by:
As far as I know python's threading module models after Java's. However, I can't find something equivalent to Java's interrupt and isInterrupted methods, along with InterruptedException....
0
by: James R. Saker Jr. | last post by:
I've got a: "start server thread > Queue object, start server thread <> Queue object, start parsing client < Queue object" application that's got me puzzled. Probably an easy threads issue, but...
77
by: Jon Skeet [C# MVP] | last post by:
Please excuse the cross-post - I'm pretty sure I've had interest in the article on all the groups this is posted to. I've finally managed to finish my article on multi-threading - at least for...
3
by: ELO | last post by:
Hi all Every week, I need to get two files on a remote server. I have developped a C# Windows Service with two System.Threading.Timer to do this task For the first one, the delay (TimeSpan...
13
by: John | last post by:
I've got some reasonably complex business logic in my C# code, in a class called by a ASP.NET page. This takes around 3-4 seconds to execute. It's not dependent on SQL calls or anything like that....
5
by: microsoft | last post by:
Just researching threading and I'm sure we need to do more review of available resources BUT I'm wondering if what we're looking for is actually possible. If it is we can keep reading. I know...
4
by: JimD | last post by:
Is this safe? Any pitfalls? I have done threading in regular C# apps, but haven't had a needs to do threading in ASP.Net, until now. The issue I have ran into is this: Our corporate portal...
2
by: WXS | last post by:
When I see things in .NET 2.0 like obsoletion of suspend/resume because of the public reason MS gives of they think people are using them inappropriately.. use mutex, monitor and other...
126
by: Dann Corbit | last post by:
Rather than create a new way of doing things: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2497.html why not just pick up ACE into the existing standard:...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
0
by: CloudSolutions | last post by:
Introduction: For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
0
by: Defcon1945 | last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work
0
by: Faith0G | last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...
0
isladogs
by: isladogs | last post by:
The next Access Europe User Group meeting will be on Wednesday 3 Apr 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM). In this session, we are pleased to welcome former...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.