Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solving Issue 249: Multi-threading issue - IndexOutOfRangeException #3

Closed
wants to merge 1 commit into from
Closed

Solving Issue 249: Multi-threading issue - IndexOutOfRangeException #3

wants to merge 1 commit into from

Conversation

FelicePollano
Copy link
Contributor

@FelicePollano FelicePollano commented Mar 22, 2012

This is the single commit to solve the issue 249 (on Google Code). There is as well an .gitihnore added and a removal of some conditional references to allow the test runs properly with XUnit gui.

@kzu
Copy link
Contributor

kzu commented May 31, 2012

Any chance that can be cleaned up so I can just merge the fix?

@FelicePollano
Copy link
Contributor Author

 I will try to fix. Problem was in the conditional references as far I remember, you confirm that the project this is  the point that make you in troubles ?
 
Da: "Daniel Cazzulino" reply@reply.github.com
A: "FelicePollano" felice@felicepollano.com
Cc:
Data: Thu, 31 May 2012 14:11:50 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Any chance that can be cleaned up so I can just merge the fix?


Reply to this email directly or view it on GitHub:
#3 (comment)

@kzu
Copy link
Contributor

kzu commented Jun 15, 2012

The more isolated the change, the easiest for me to merge even from the website without having to even compile ;).

@kzu
Copy link
Contributor

kzu commented Sep 21, 2012

Felice: did you get a chance to work this one out? It would be great to have your fix merged in!

Thanks in advance.

@FelicePollano
Copy link
Contributor Author

 I need to learn using GIT, I did a mess last time. I think I had a chance to work on it Monday, but I need some help/advice from you or from someone else leading the project. Can you keep in touch?
Maybe on twitter ? @FelicePollano

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc:
Data: Fri, 21 Sep 2012 12:08:18 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Felice: did you get a chance to work this one out? It would be great to have your fix merged in!

Thanks in advance.

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@FelicePollano
Copy link
Contributor Author

 Done!
You should have a nice pull request easy to merge. 
I added a unit test following the issue pattern you already use in your project.
The core modification i sjust a single lock in interceptor.cs. Git difference engine seems to perceive this as a whole function removal and addition with just two lines modified :(
I've no control over this, I really van't make it 'nicer', but don't be scared by the whole function red: is just actually two lines. 
Hope you can confortably pull.
Please accept my compliments fro writing MoQ, a reaggly great and smart tool.
If you like my contribution, I will try to cooperate with the team in issue solving.
regards,

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc:
Data: Fri, 21 Sep 2012 12:08:18 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Felice: did you get a chance to work this one out? It would be great to have your fix merged in!

Thanks in advance.

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@kzu
Copy link
Contributor

kzu commented Sep 25, 2012

Thanks a bunch for taking the time!

The cause for add/remove "errors" is formatting most of the time. Moq uses
tabs. If you have VS set to use whitespaces, then that happens. Is that the
case here?

/kzu

Daniel Cazzulino

On Mon, Sep 24, 2012 at 4:51 AM, Felice Pollano notifications@github.comwrote:

Done!
You should have a nice pull request easy to merge.
I added a unit test following the issue pattern you already use in your
project.
The core modification i sjust a single lock in interceptor.cs. Git
difference engine seems to perceive this as a whole function removal and
addition with just two lines modified :(
I've no control over this, I really van't make it 'nicer', but don't be
scared by the whole function red: is just actually two lines.
Hope you can confortably pull.
Please accept my compliments fro writing MoQ, a reaggly great and smart
tool.
If you like my contribution, I will try to cooperate with the team in
issue solving.
regards,

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc:
Data: Fri, 21 Sep 2012 12:08:18 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue -
IndexOutOfRangeException (#3)

Felice: did you get a chance to work this one out? It would be great to
have your fix merged in!

Thanks in advance.

—>
Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-8809735.

@FelicePollano
Copy link
Contributor Author

 Yeah is the tab/spaces :(,
Sadly I suppose this tab/spaces fact was the one misleading me on the first time too.
I fixed the issue in the request, now the resulting merge will be moq compliant. 
Thanks a lot,
Felix

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Tue, 25 Sep 2012 07:37:10 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Thanks a bunch for taking the time!

The cause for add/remove "errors" is formatting most of the time. Moq uses

tabs. If you have VS set to use whitespaces, then that happens. Is that the

case here?

/kzu

Daniel Cazzulino

On Mon, Sep 24, 2012 at 4:51 AM, Felice Pollano notifications@github.comwrote:

Done!

You should have a nice pull request easy to merge.

I added a unit test following the issue pattern you already use in your

project.

The core modification i sjust a single lock in interceptor.cs. Git

difference engine seems to perceive this as a whole function removal and

addition with just two lines modified :(

I've no control over this, I really van't make it 'nicer', but don't be

scared by the whole function red: is just actually two lines.

Hope you can confortably pull.

Please accept my compliments fro writing MoQ, a reaggly great and smart

tool.

If you like my contribution, I will try to cooperate with the team in

issue solving.

regards,

Felice Pollano

Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com

A: "Moq/moq4" moq4@noreply.github.com

Cc:

Data: Fri, 21 Sep 2012 12:08:18 -0700

Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue -

IndexOutOfRangeException (#3)

Felice: did you get a chance to work this one out? It would be great to

have your fix merged in!

Thanks in advance.

—>

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-8809735.

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@mcintyre321
Copy link

Do you know when this will be pushed to Nuget? We've just run into this issue today - it's amazing watching it get fixed in realtime!

@FelicePollano
Copy link
Contributor Author

 I hope soon. Is not up to me but up to Daniel or someon else  leading the project.
Actuyally I fix this one year ago, but there was something wrong in my pull requests so the reason we are late.

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Harry McIntyre" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Tue, 25 Sep 2012 09:00:32 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Do you know when this will be pushed to Nuget? We've just run into this issue today - it's amazing watching it get fixed in realtime!

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@RedwoodForest
Copy link

We are also running into this issue and are looking forward to the fix being pushed to NuGet.

@RedwoodForest
Copy link

Any update on when this will be pushed to NuGet?

@FelicePollano
Copy link
Contributor Author

@lawrencejonston It is pushed in the Dev branch.

@FelicePollano
Copy link
Contributor Author

 It is pushed in the Dev branch.

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Lawrence Johnston" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Tue, 05 Feb 2013 16:59:20 -0800
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

Any update on when this will be pushed to NuGet?

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@shsomash
Copy link

shsomash commented Feb 7, 2013

We are invoking a method on a mock object on several threads in parallel. When we try verifying the number of calls, it almost always blows up, with a null reference exception or out of range exception. Works fine, if the calls are in sequence.

It appears, the code used by Moq to count the number of invocations is not thread-safe. It is also not clear if the above fix addresses this issue. If not, please let me know, so I can file a bug.

Thanks,
Shiv

1 similar comment
@shsomash
Copy link

shsomash commented Feb 7, 2013

We are invoking a method on a mock object on several threads in parallel. When we try verifying the number of calls, it almost always blows up, with a null reference exception or out of range exception. Works fine, if the calls are in sequence.

It appears, the code used by Moq to count the number of invocations is not thread-safe. It is also not clear if the above fix addresses this issue. If not, please let me know, so I can file a bug.

Thanks,
Shiv

@FelicePollano
Copy link
Contributor Author

 This is solved in teh .Dev branch. You can download and compile. If even in this case you have the problem, please produce an unit test to show the behavior.

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

From: "shsomash" notifications@github.com
To: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Date: Thu, 07 Feb 2013 07:41:57 -0800
Subject: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

We are invoking a method on a mock object on several threads in parallel. When we try verifying the number of calls, it almost always blows up, with a null reference exception or out of range exception. Works fine, if the calls are in sequence.

It appears, the code used by Moq to count the number of invocations is not thread-safe. It is also not clear if the above fix addresses this issue. If not, please let me know, so I can file a bug.

Thanks,>
Shiv

        > 
          —> 
          Reply to this email directly or view it on GitHub.

@ftardif
Copy link

ftardif commented Mar 20, 2013

Is this fix suppose to be available in the current nuget Moq 4.0.10827 ?

@kzu
Copy link
Contributor

kzu commented Mar 20, 2013

No :(

If anyone wants to contribute by automating the nuget generation, and then
the automated publishing via myget, just me know. I don't have the
bandwidth other than to review and approve pull requests (usually via the
browser).

Thanks!

/kzu from mobile
On Mar 20, 2013 6:27 PM, "Frederic Tardif" notifications@github.com wrote:

Is this fix suppose to be available in the current nuget Moq 4.0.10827 ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-15204726
.

@ftardif
Copy link

ftardif commented Mar 21, 2013

I tried the patch but I was still getting the same issue. While digging in the code, the racing codition pops a concurrent modification exception when looping through the InterceptStrategyContext.OrderedCalls in ExtractProxyCall#handle intercept (InterceptorStrategy.cs line 103). The access is well within the lock boundary of the Interceptor#intercept, but concurrently, the orderedCalls list is modified (add, delete) in the Interceptor#AddCall without being synchronized on the Mock. If I add lock(Mock) in the Interceptor#AddCall before either removing or adding the orderedCalls list, it fixes the issue.

Frederic

@FelicePollano
Copy link
Contributor Author

 I would like to help.
Is ok to create a NuGet package with the current dev trunk?
Otherwise can we push all current dev modifications to the main trunk ?

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Wed, 20 Mar 2013 16:52:37 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

No :(

If anyone wants to contribute by automating the nuget generation, and then

the automated publishing via myget, just me know. I don't have the

bandwidth other than to review and approve pull requests (usually via the

browser).

Thanks!

/kzu from mobile

On Mar 20, 2013 6:27 PM, "Frederic Tardif" notifications@github.com wrote:

Is this fix suppose to be available in the current nuget Moq 4.0.10827 ?

Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-15204726

.

—> Reply to this email directly or view it on GitHub.

@FelicePollano
Copy link
Contributor Author

 @frederic If you can provide a test reproducing the problem, I will try to have a look at it.

 

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Frederic Tardif" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Wed, 20 Mar 2013 19:38:56 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue - IndexOutOfRangeException (#3)

I tried the patch but I was still getting the same issue. While digging in the code, the racing codition pops a concurrent modification exception when looping through the InterceptStrategyContext.OrderedCalls in ExtractProxyCall#handle intercept (InterceptorStrategy.cs line 103). The access is well within the lock boundary of the Interceptor#intercept, but concurrently, the orderedCalls list is modified (add, delete) in the Interceptor#AddCall without behind synchronized on the Mock. If I add lock(Mock) in the Interceptor#AddCall before either removing or adding the orderedCalls list, it fixes the issue.

Frederic

—> Reply to this email directly or view it on GitHub.

@kzu
Copy link
Contributor

kzu commented Mar 21, 2013

Thanks Felice!

We'd need a build.bat at the root of the source that produces the .nupkg.
Similar to what I'm doing at github.com/clariuslabs/adapter.

With that, we can then configure myget.org to clone&build on every commit
(I think that is done already, but there is no nupkg output currently).
This would pull from dev, and another build would do the same from main.

With that in place, pushing a fix/feature to nuget.org is a matter of:

  • contribute and send pull request against dev
  • we review and merge
  • once we want to make it public, merge to main
  • wait for myget.org to finish
  • one click on myget to push to nuget. Org

And everyone will always have a dev build package from dev by using the
"CI" feed directly from myget.org instead of waiting for the public stable
release on nuget.org

/kzu from mobile
On Mar 21, 2013 5:06 AM, "Felice Pollano" notifications@github.com wrote:

I would like to help.
Is ok to create a NuGet package with the current dev trunk?
Otherwise can we push all current dev modifications to the main trunk ?

Felice Pollano
Senior IT Architect & Developer

http://www.felicepollano.com

Da: "Daniel Cazzulino" notifications@github.com
A: "Moq/moq4" moq4@noreply.github.com
Cc: "Felice Pollano" felice@felicepollano.com
Data: Wed, 20 Mar 2013 16:52:37 -0700
Oggetto: Re: [moq4] Solving Issue 249: Multi-threading issue -
IndexOutOfRangeException (#3)

No :(

If anyone wants to contribute by automating the nuget generation, and then

the automated publishing via myget, just me know. I don't have the

bandwidth other than to review and approve pull requests (usually via the

browser).

Thanks!

/kzu from mobile

On Mar 20, 2013 6:27 PM, "Frederic Tardif" notifications@github.com
wrote:

Is this fix suppose to be available in the current nuget Moq 4.0.10827 ?

Reply to this email directly or view it on GitHub<
https://github.com/Moq/moq4/pull/3#issuecomment-15204726>

.

—> Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-15223915
.

@asmus
Copy link

asmus commented Apr 23, 2013

We ran into the same issue and decided to build our own version from the DEV branch. Unfortunately this didn't work so well. We dont't have the NullReferenceException anymore, but instead we got a Deadlock now. I was able to reproduce the situation with a minimal test case:

public class ClassToMock
{
    AutoResetEvent reset = new AutoResetEvent(false);
    public virtual void M1()
    {
        var task = new TaskFactory().StartNew(M2);
        Thread.Sleep(500);
        reset.Set();
        task.Wait();
    }

    public virtual void M2()
    {
        reset.WaitOne();
    }
 }

[TestMethod]
public void TestMock()
{
    var testMock = new Mock<ClassToMock>{CallBase = true};
    testMock.Object.M1();
    testMock.Verify(x => x.M1());
    testMock.Verify(x => x.M2());
}

This will block forever with the current DEV branch version. It this already known? Or do I miss something obvious here?

@asmus asmus mentioned this pull request Apr 25, 2013
@kzu
Copy link
Contributor

kzu commented Apr 25, 2013

@FelicePollano I'd be greateful if you could look into this issue :)

ppittle added a commit to ppittle/moq4 that referenced this pull request Oct 10, 2015
david-kalbermatten added a commit to david-kalbermatten/moq4 that referenced this pull request Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants