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

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

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

"Marc Gravell" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> 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 <(E-Mail Removed)> wrote:
> 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.

--
Jon Skeet - <(E-Mail Removed)>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

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

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]" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> Kristijan Marin <(E-Mail Removed)> wrote:
>> 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.
>
> --
> Jon Skeet - <(E-Mail Removed)>
> http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
> World class .NET training in the UK: http://iterativetraining.co.uk

Kristijan Marin <(E-Mail Removed)> wrote:
> 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?

--
Jon Skeet - <(E-Mail Removed)>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

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);
}
}




"Marc Gravell" <(E-Mail Removed)> wrote in message
news:%237e%(E-Mail Removed)...
> 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
>

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 ...


"Jon Skeet [C# MVP]" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> Kristijan Marin <(E-Mail Removed)> wrote:
>> 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?
>
> --
> Jon Skeet - <(E-Mail Removed)>
> http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
> World class .NET training in the UK: http://iterativetraining.co.uk