Monitor Mayhem

  • Thread starter Thread starter C# Learner
  • Start date Start date
C

C# Learner

My bug's back. It's reappeared even though I haven't made any changes
since.

Just to check: is the use of locking in the following okay? Note that
'connections' and 'activeConnections' both reference instances of
'ArrayList'.

void connection_Drop(object sender, bool allSent)
{
lock (connections) {
connections.RemoveAt(connections.IndexOf(sender));
}

lock (activeConnections) {
int i = activeConnections.IndexOf(sender);
if (i != -1) {
activeConnections.RemoveAt(i);
ClientTransferEnd(this, i, allSent);
}
}
}

Here it is without K&R bracing style:

void connection_Drop(object sender, bool allSent)
{
lock (connections)
{
connections.RemoveAt(connections.IndexOf(sender));
}

lock (activeConnections)
{
int i = activeConnections.IndexOf(sender);
if (i != -1)
{
activeConnections.RemoveAt(i);
ClientTransferEnd(this, i, allSent);
}
}
}
 
C# Learner said:
My bug's back. It's reappeared even though I haven't made any changes
since.

Just to check: is the use of locking in the following okay? Note that
'connections' and 'activeConnections' both reference instances of
'ArrayList'.
Click to expand...

Well, it really depends on what ClientTransferEnd does. Otherwise, it
looks okay - although I'd either use connections.SyncRoot and
activeConnections.SyncRoot, or a separate variable used only for
locking.
 
Jon said:
Well, it really depends on what ClientTransferEnd does.
Click to expand...

'ClientTransferEnd' is an event which is handled by a method in a form
class. This form class method then marshalls a call to the GUI thread.

For what it's worth, here's that code:
http://sourcepost.sytes.net/sourceview.aspx?source_id=14487

'Server_ClientTransferEnd' handles the call to 'ClientTransferEnd'.

Obviously I understand that anyone reading this cannot debug this code
without seeing what the logic presented depends on, but I decided to
post it in case someone picks up on something that, on its own, looks dodgy.
Otherwise, it
looks okay - although I'd either use connections.SyncRoot and
activeConnections.SyncRoot, or a separate variable used only for
locking.
Click to expand...

Okay, I've read about SyncRoot, but I can't find out the difference
between using [lock (arrayListInstance)] and [lock
(arrayListInstance.SyncRoot)] unless enumerating the ArrayList is
involved (which isn't in my case).

In any case, I've tried using ArrayList.SyncRoot and have also tried
using a separate variable for locking, to no avail.

Thanks for your reply.
 
C# Learner said:
'ClientTransferEnd' is an event which is handled by a method in a form
class. This form class method then marshalls a call to the GUI thread.

For what it's worth, here's that code:
http://sourcepost.sytes.net/sourceview.aspx?source_id=14487

'Server_ClientTransferEnd' handles the call to 'ClientTransferEnd'.
Click to expand...

Ah. So you've got a lock on activeConnections, but then you're waiting
until ClientTransferEnd finishes. I can't see anything in the GUI
thread which would need activeConnections, but it seems like a bad
idea. Why do you have it in the locked section?

Otherwise, it
looks okay - although I'd either use connections.SyncRoot and
activeConnections.SyncRoot, or a separate variable used only for
locking.
Click to expand...

Okay, I've read about SyncRoot, but I can't find out the difference
between using [lock (arrayListInstance)] and [lock
(arrayListInstance.SyncRoot)] unless enumerating the ArrayList is
involved (which isn't in my case).
Click to expand...

It's really in case you use ArrayList.Synchronized, I believe - that
wil create another version of the ArrayList (a proxy, really) - and
that will lock on the SyncRoot of the original, I believe.
In any case, I've tried using ArrayList.SyncRoot and have also tried
using a separate variable for locking, to no avail.
Click to expand...

Well, see whether the above helps you. If you can come up with a short
but complete program which demonstrates the problem, you know how much
I love them :)
 
Jon Skeet [C# MVP] wrote:

[...]
Ah. So you've got a lock on activeConnections, but then you're waiting
until ClientTransferEnd finishes. I can't see anything in the GUI
thread which would need activeConnections, but it seems like a bad
idea. Why do you have it in the locked section?
Click to expand...

Well, there are three categories of threads used in this program:

a) the GUI thread;
b) the thread in which the server listens for connections; and
c) threads in which communication (reading/writing) with the clients
takes place.

'activeConnections' can be accessed by multiple different (c) threads
(in event handlers which are fired when the client makes a request and
when client drops).

Actually, 'activeConnections' should be renamed to something like
'connectionsInWhichClientHasMadeARequest'.

Anywho...

[...]
Okay, I've read about SyncRoot, but I can't find out the difference
between using [lock (arrayListInstance)] and [lock
(arrayListInstance.SyncRoot)] unless enumerating the ArrayList is
involved (which isn't in my case).
Click to expand...

It's really in case you use ArrayList.Synchronized, I believe - that
wil create another version of the ArrayList (a proxy, really) - and
that will lock on the SyncRoot of the original, I believe.
Click to expand...

Oh, okay. It's a pity that the MSDN documentation is so minimal here.
Well, see whether the above helps you. If you can come up with a short
but complete program which demonstrates the problem, you know how much
I love them :)
Click to expand...

Okay, I'll try making a copy of the project and removing everything
unrelated to the problem; that'd probably be the easiest way here. If I
can do this, I'll send it here.

Thanks!
 
Jared said:
What is your bug?
Click to expand...

I should've given a reference to "my bug" (which would be the thread
'All Your Thread Are Belong To Us'). Sorry about that.

It's fixed now (as can be seen in 'Threading Problem Demonstration
Program (contains attachment)').
 
Back
Top