Could Application.DoEvents(); messup code execution ??

[Kristijan Marin]

Hi,

I have experianced same wierd behavior by one of my customers .....

Here is the thing .... We have the application for newspaper article
management. so adding end editing etc ....

After a while my customer started to complain that they have duplicate
articles in there database ....

so I put some traces on the save event ..... and what I could find out is
that the event method OnClick for save ...

was called 2 or even 3 times in a row ....??? How is this possible ?? What
is even more wierd is that this happens maybe 1 or 2 times from 2500 !!!

I also put some traces when entering the method and exiting the method .....
and I get 2 times enter 1 time exit ?:???

So there can't be error in code .... Ofcourse I COUDN'T repeat this error
not even ones ....

Now the only difference from my PC to customers is that I have .NET
Framework 2.0 SP1 installed and they don't .....

What I did put in the code to force refresh of other processes is :

Application.DoEvents();
Thread.Sleep(300);

Was this wrong ?? Could this mess-up the code execution ??


Another thing that happened was when they tried to edit the article and same
it .... and because they changed the media code .... thumb generation
mechanizem puts
new line in oracle table so that the background process creates new thumb
.....

And what happened was that the code (see bellow) was somehow skiped .....
well I can't see if it was skipped or not, but there was 100% no new line
for thumb generation

void OnClick(...)
{
// some adding and updating of the article
Application.DoEvents();
Thread.Sleep(300);

if (old_media != new_media)
{
///do our table insert for thumbs .....
}
}

So as you can see i called this :
Application.DoEvents();
Thread.Sleep(300);

ones more here in this code .... so maybe this is a problem.....

Would anyone have any clue what is wrong here .

Thanks a lot.
Kris

 

[Marc Gravell]

Well, the Sleep isn't doing you any favors... you are on the same thread -
you are just slowing it down (assuming there isn't any other threading going
on).

DoEvents is notorious for causing re-entrancy issues exactly as you have
described. If you are doing background work, I would recommend a worker
thread, but this isn't convenient if you are talking to the UI lots (only
the UI thread can talk to the UI controls; the worker thread would have to
do lots of marshalling).

In this case, it sounds like the most pragmatic thing to do would be to add
a flag (either a bool or an int counter) to prevent re-entrancy:

bool inProgress;
void Some_Click(object sender, EventArgs args) {
if(inProgress) return; // dammit I heard you already! stop clicking!
inProgress = true;
try {
// your current code
} finally {
inProgress = false;
}
}

Marc

 

[Kristijan Marin]

Hi,

The flag would not exactly do the job, cause as i described .... I could see
that entering method was done twice , didn't set trace on exit but Commit on
DB was executed only ones ..I would expect Commit to be executed twice too..
and that is the second time I presume ... no Rollback was executed ... cause
if it would then i would know and no RETURN was neither executed .....

Looks just like the code has lost it self somewhere in the middle .... and
as I said in 1 or 2 examples form 2500.... so it's really hard to track ....

but if I can recall i thing that all this stuff started to happen when I
included DoEvents() ..... so now I'll remove all that from code and hope for
the best ...

Thanks,
Kris

 

[Jon Skeet [C# MVP]]

The flag would not exactly do the job, cause as i described .... I could see
that entering method was done twice , didn't set trace on exit but Commit on
DB was executed only ones ..I would expect Commit to be executed twice too..
and that is the second time I presume ... no Rollback was executed ... cause
if it would then i would know and no RETURN was neither executed .....

Looks just like the code has lost it self somewhere in the middle .... and
as I said in 1 or 2 examples form 2500.... so it's really hard to track ....

but if I can recall i thing that all this stuff started to happen when I
included DoEvents() ..... so now I'll remove all that from code and hope for
the best ...

That's not a reassuring way of coding.

You shouldn't be doing DB work in the UI thread, basically.
Application.DoEvents() is typically used to make the UI responsive
during a long-running activity - but threads are a *much* better way of
doing that.

Unfortunately, threading is not an easy topic.
See http://pobox.com/~skeet/csharp/threads
for an introduction.

 

[Marc Gravell]

Well, you haven't posted any code showing the commit etc... so I can't
comment. But I suspect the flag *would* work. You are entering and exiting
the method the right number of times - just not in the order you expect ;-p

What you are probably seeing is

Enter Method
[do events]
Enter Method
Exit Method
[return from do events]
Exit Method

or some even worse combinations; you want to stop that middle code from
happening... the easiest option is a short-circuit flag - or: don't use
DoEvents!

Marc

 

[Kristijan Marin]

If I just do some INSERT into DB ... why shouldn't I do this in
OnButtonClick method ? Don't thing anyone is using additional threads to
store data into db that takes less then 1 second for processing ... Do they
?
At least I don't know anyone ....

Thanks for replies.

Kris

 

[Jon Skeet [C# MVP]]

If I just do some INSERT into DB ... why shouldn't I do this in
OnButtonClick method ? Don't thing anyone is using additional threads to
store data into db that takes less then 1 second for processing ... Do they
?
At least I don't know anyone ....

Try doing it when your database is running slowly, or there's a network
connectivity issue so it times out...

If it's all working quickly for you, why did you include
Application.DoEvents in the first place?

 

[Kristijan Marin]

Here is the flow and there is no error so let's presume everything is ok
so rollback is never called so no MESSAGEBOX is showen either :

Below is the code .... what I forgot to mention before was that .....

no matter that method is entered 2 times so m_addNr =2 and m_clickNr =2

and you can see that i have a line "DateTime currentDate = DateTime.Now;"
which gets current date & time ....

and I execute INSERT into DB where i put this DATETIME as one of input
parameters ....

when I GET duplicate record , the time is EXACTLY the SAME for BOTH records
!! ?? Isn't that wierd ??? Exactly the same to millisecond !!

So this could then only mean that the "b_add_Click" method had to be called
or executed in the same time cause otherwise "DateTime.Now" would return
diffrent time at least on milliseconds, right ?? And if this is true , why
is then COMMIT called only once ?

Thank you guys for everything .....



void method()
{
!!!!// this point here is entered 2 times
if (CheckAttributes)
{
!!!!// this point here is entered 2 times
///insert into ARTICLE tabel
/// insert into ATTACHMENT table

if (bool_everythingIsINSERTED_OK)
{
!!!!// this point here is entered 1 time
commit.
closeForm();
return;
}
else
{
rollback
MessageBox.
return;
}
}
}


and here the code (i just removed some variables and some stuff )


private void b_add_Click(object sender, EventArgs e)
{
bool isError = false;
try
{
m_addNr++; !!!!!!!!!!!!!!!!! HERE COUNTER INCREMENTS so I
know how many times the method was entered !!!!!
if (RecordCheck())
{

long newID =
Globals.m_sqlConn.getNextSeq("MS_ARTICLE_SEQ", ref _err);
m_clickNr++; !!!!!!!!!!!!!!!!!!!!!!!!!!! HERE
COUNTER INCREMENTS so I know how many times code execusion was passed the
RecordCheck and got new SEQUENCE

if (newID == -1)
{
MessageBox.Show(this, _err,
CommonDll.Localization.dlg_error_caption, MessageBoxButtons.OK,
MessageBoxIcon.Error);
Globals.m_sqlConn.Rollback();
return;
}

DateTime currentDate = DateTime.Now; !!!!!!!! THIS
DATE IS A PANDORA BOX !!!!!!!
if (m_COORD_TEXT.Count > 0)
{
sql = "INSERT INTO MS_ARTICLE (ID,TITLE,SUBTITLE
,ART_DATE ,CREATE_DATE,MEDIA_CODE ,SHOW_TEXT,SUBCATEGORY,GROUP1,SUBGROUP," +
}
else
{
sql = "INSERT INTO MS_ARTICLE (ID,TITLE,SUBTITLE
,ART_DATE ,CREATE_DATE,MEDIA_CODE ,SHOW_TEXT,SUBCATEGORY,GROUP1,SUBGROUP," +
}
#region Collect Attachments
#endregion

if (Globals.m_sqlConn.InsertWithCLOB4(sql, "zones",
"attachments", "fulltext", "coords", m_ZONES, m_ATTACHMENTS, FULLTEXT,
COORD, ref _err) == 0)
{
string err = "Error:" + _err;
MessageBox.Show(this, err,
CommonDll.Localization.dlg_error_caption, MessageBoxButtons.OK,
MessageBoxIcon.Error);
Globals.m_sqlConn.Rollback();
return;
}

if (m_clickNr > 1)
{
Globals.LogActions("OK", "CONTROL_MNG",
Globals.UserAccess_UserName, "", "", "Duplicate article possible. ID:" +
newID + " C:" + m_clickNr + " A:" + m_addNr+" COM:"+m_commitNr);
Globals.MessageBox_Info("This article
can be a possible duplicate. ID: " + newID);
}

Globals.LogActions("OK", "CONTROL_MNG",
Globals.UserAccess_UserName, "", "", "Adding article." + newID);
Globals.m_sqlConn.Commit();
this.DialogResult = DialogResult.OK;
m_commitNr++; //////!!!!!!!!!!!!!!!!! HERE
COUNTER INCREMENTS so I know how many times COMMIT was executed !!!!
this.Close();
}
}
}
}
catch (Exception ex)
{
Globals.m_sqlConn.Rollback();
MessageBox.Show(this, ex.Message,
CommonDll.Localization.dlg_error_caption, MessageBoxButtons.OK,
MessageBoxIcon.Error);
}
}

Well, you haven't posted any code showing the commit etc... so I can't
comment. But I suspect the flag *would* work. You are entering and exiting
the method the right number of times - just not in the order you expect
;-p

What you are probably seeing is

Enter Method
[do events]
Enter Method
Exit Method
[return from do events]
Exit Method

or some even worse combinations; you want to stop that middle code from
happening... the easiest option is a short-circuit flag - or: don't use
DoEvents!

Marc

 

[Kristijan Marin]

I had one Progressbar control attached on the same GUI which was not run in
another thread ....
So I thought that DoEvents would refresh the drawing of this control ....
that's the only reason I
included DoEvents in the code in the first place.... and i didn't want to
create BackgroundThread for that ...