do i need monitors and locking?

[Guest]

hi all,

im trying to establish whether i have a race condition or critical section
in the following. i have a dataaccess class that continually retireves a
table from a sqlserver (which may be slow). it provides a cached copy of
this table to clients, for them to read from (note: there are no writes).
the idea being if a particular table retrival takes time, the class would
still provide a cached copy.

now the code:

public abstract class DataAccess
{

protected DataTable cached;


protected int repeattime;

private DataSet ds;

public UpdatableDataAccess(string s, string db):base(s, db)

{

//create connection initialise dataset

}

protected DataTable fillDataSet(string tablename, SqlCommand sqlcom)

{

SqlDataAdapter mysda = new SqlDataAdapter(sqlcom);

if (ds.Tables.Contains(tablename))

{

ds.Tables[tablename].Clear();

}

sqlc.Open();

mysda.Fill(ds, tablename);

sqlc.Close();

return ds.Tables[tablename].Copy();

}

public void updateTable()

{

while(true)

{

SqlCommand sqlcom = new SqlCommand(updatequery, sqlc);

DataTable holding = fillDataSet(tablename, sqlcom);

//critical section, i think

cached = holding;

//end critical

Thread.Sleep(repeattime * 1000);

}

}

public DataTable getLatestTable()

{

//critical section, i think

return cached;

//end critical

}

}



have i indicated the correct critical sections? if so, do i need to lock /
tryenter around them? i assume that the copy assignment to the cached field
is atomic, so there shouldnt be any problem, right? clients would use
"getLatestTable()" to get a table, while the thread method would be
"updateTable()". this is my first real MT app, so i think im missing
something!



Spammy

 

[Guest]

Hi all,

Further to my multithreading question from yesterday, it seems that,
although assignments are atomic, field values may be cached within threads
and so its still worth using locking to force reads from memory. However, I
also read that declaring a field as volatile would do this too (since
locking actaully uses volatile read and writes).

Its not much difference in work implementing either, but which is the best
solution? Volatile is said to have some unusual semantics, but im not sure
what these are. Locking OTOH, seems (to me at least) to be a bit of an
overkill for a simple assignment or read...

Thanks!

Spammy

 

[Jon Skeet [C# MVP]]

Further to my multithreading question from yesterday, it seems that,
although assignments are atomic, field values may be cached within threads
and so its still worth using locking to force reads from memory. However, I
also read that declaring a field as volatile would do this too (since
locking actaully uses volatile read and writes).

Yes it will.

Its not much difference in work implementing either, but which is the best
solution? Volatile is said to have some unusual semantics, but im not sure
what these are. Locking OTOH, seems (to me at least) to be a bit of an
overkill for a simple assignment or read...

Personally I use locks everywhere, and never bother with volatile. That
means that if I change to write more than one field, I don't have to
rething things. Likewise not all types can be used with volatile
variables.

If you put the locking code within property access/mutate, you don't
need to have locks all over the place in your code - just use the
property instead.

I would recommend *not* locking on "this"/"Me" though - you should
generally try to lock on a reference which other classes don't have
access to.

I'm writing a multi-threading article which covers a lot of this, but
I'm afraid it won't be ready for a while - it's a big topic!