Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Dec 11 21:41
    jeremydmiller commented #583
  • Dec 11 21:27
    jeremydmiller commented #583
  • Dec 10 15:45
    jeremydmiller labeled #585
  • Dec 10 15:44
    jeremydmiller edited #556
  • Dec 07 22:08
    jeremydmiller commented #583
  • Dec 07 21:29
    jeremydmiller edited #583
  • Dec 07 21:29
    jeremydmiller closed #580
  • Dec 07 21:29
    jeremydmiller commented #580
  • Dec 07 21:29
    jeremydmiller demilestoned #580
  • Dec 07 14:39
    jeremydmiller demilestoned #562
  • Dec 07 14:39
    jeremydmiller milestoned #562
  • Dec 07 12:51
    jeremydmiller commented #562
  • Dec 06 20:45
    jeremydmiller closed #574
  • Dec 06 20:45

    jeremydmiller on master

    Better Disposal mechanics to ge… (compare)

  • Dec 06 19:47
    jeremydmiller closed #582
  • Dec 06 19:47
    jeremydmiller commented #582
  • Dec 06 19:47

    jeremydmiller on master

    Little rename and method access… Local routing improvements, new… (compare)

  • Dec 06 18:45
    jeremydmiller closed #563
  • Dec 06 18:45

    jeremydmiller on master

    New Endpoint.ReplyUri() method New Endpoint.ReplyEndpoint() me… Little tweak to ReplyUri mechan… (compare)

  • Dec 06 18:44
    jeremydmiller commented #563
Jeremy D. Miller
@jeremydmiller
And nothing gets disposed, correct?
Mark Warpool
@CodingGorilla
No, the repository gets disposed while I'm using it
Jeremy D. Miller
@jeremydmiller
How? Are you taking care of that yourself somehow in your own code?
Mark Warpool
@CodingGorilla
No, this new handler (the DeleteOwnerAccountHandler) is just throwing all kinds of ObjectDisposedExceptions and I don't know why, in stepping through the code I can see the repository being disposed in the middle of the handle method execution
but the other handler, which is set up almost identically (UpdateFeatureSubscriptionsHandler) has been working fine (not sure if it still is, but I assume it is)
Jeremy D. Miller
@jeremydmiller
I’m sorry Mark, I just don’t see enough here to help you much.
Mark Warpool
@CodingGorilla
No problem, I thought I had caught it with that elided task, but I guess that's not it.. I'll keep digging
Mark Warpool
@CodingGorilla
arg... no it is the task eliding issue
the code gen came out in a different order the second time around, so that was the runReportHandler that I saw without the elided task lol
Jeremy D. Miller
@jeremydmiller
As a quick fix, I’ll push a Jasper version really soon that just forces it to always choose async/await
Mark Warpool
@CodingGorilla
I can work around it by using the repository factory, that is specifically design to avoid using any containers at all for exactly this sort of reason
so if you want to spend more time "doing it right" that's fine with me
Jeremy D. Miller
@jeremydmiller
We can introduce a custom Instance for the repository contruction that would take care of that as well though
Mark Warpool
@CodingGorilla
To replace my factory you mean?
Jeremy D. Miller
@jeremydmiller
Naw, to let codegen work even with the repository registration. So you just act for a repository, and the codegen handles it all for you.
Mark Warpool
@CodingGorilla
oh, right, that would be ideal =)
Mark Warpool
@CodingGorilla
ack... I can work around the issue with the factory, but it's ugly as hell, cuz I can't using IDisposable on the message handler, because that ends up getting called out of sync too, so I have to keep everything locally...
let me see if I can throw together a pull request for you
I know you've been busy at work lately :)
Jeremy D. Miller
@jeremydmiller
Yeah, not sure when that’s going to improve. It might be in the LamarCodeGeneration stuff. Needs to somehow do a look above in the frame tree to know if anything wraps it in a using block
I swear I’ve tried to deal with this before, but that would have been ~2 years ago and I’ve slept since then
Mark Warpool
@CodingGorilla
yea, looks like it's in LamarCodeGeneration
Mark Warpool
@CodingGorilla
So there seems to be a few different places where fixing this issue could be done, lamar's code gen seems to lean on the AsyncMode setting on the GeneratedMethod, but it also never seems to actually set that. So to make it "smarter" it would need to lean a little more on computing what the async mode should be.
So I'm happy to work on a PR for you, but I would need a little guidance from you on how to implement it. At which point, maybe you just would rather do it yourself.
Jeremy D. Miller
@jeremydmiller
It should be derived by looking through the Frames
Mark Warpool
@CodingGorilla
@jeremydmiller Ok, so after a lot of digging, and wracking my brains on how to successfully write a failing unit test for this, it looks like the fix is fairly simple. It looks like just changing the ServiceScopeFactoryCreation to set IsAsync to true would solve the problem, even though that is technically not true.
Also, it derives from SyncFrame, which always sets it to false, but doesn't provide anything else, so I'm not sure if there's some significance to making it a SyncFrame.
But in my hacky little test, making that simple change solves the problem
Jeremy D. Miller
@jeremydmiller
Ugh, that’s not quite right though. It’s possible to use that frame and not have any async. Is there a prop like IsWrapped or something liek that, so the actual method call from would know that there’s a reason to use async/await
?
Mark Warpool
@CodingGorilla
Oh, good point. Let me look into that
Mark Warpool
@CodingGorilla

Ok, so my thought is to add a new property to Frame: RequiresAwait, and then modify the MethodFrameArranger.Arange() as:

public void Arrange(out AsyncMode asyncMode, out Frame topFrame)
{
    var compiled = compileFrames(_method.Frames);

    asyncMode = AsyncMode.AsyncTask;

    if(compiled.Any(x => x.IsAsync) && compiled.Any(x => x.RequiresAwait))
    {
        asyncMode = AsyncMode.AsyncTask;
    }
    else if (compiled.All(x => !x.IsAsync))
    {
        asyncMode = AsyncMode.None;
    }
    else if (compiled.Count(x => x.IsAsync) == 1 && compiled.Last().IsAsync && compiled.Last().CanReturnTask())
    {
        asyncMode = compiled.Any(x => x.Wraps) ? AsyncMode.AsyncTask : AsyncMode.ReturnFromLastNode;
    }

    topFrame = chainFrames(compiled);
}

Thoughts?

Jeremy D. Miller
@jeremydmiller
I don’t think you need a new property on the frame. The IsAsync should be enough.
The case that screwed us is if ONLY the very last frame is IsAsync == true, but there’s another Frame somewhere that’s Wraps == true.
Mark Warpool
@CodingGorilla
well, if you want to just naively force an await on anything that is async, with this though, it would be "smarter"
Jeremy D. Miller
@jeremydmiller
I wonder if the Frame that wrote out the using statement was giving you the right Wraps value
It solves your problem fast. Returning the last task was just more optimal performance wise where you could get away with it
Mark Warpool
@CodingGorilla
what is Wraps indicative of?
Jeremy D. Miller
@jeremydmiller
That the frame does something that “wraps” the code in the following frames. So in the case of a using block, the prior Frame wraps code around the later frames
Mark Warpool
@CodingGorilla
The frame that caused the using was the ServiceScopeFactoryCreation, oh, and so yea it sounds like the ServiceScopeFactoryCreation should set Wraps to true, but it doesn't
So let me just change that quickly and see if that fixes the test
Jeremy D. Miller
@jeremydmiller
:thumbsup:
Mark Warpool
@CodingGorilla
HAHA, I spent like 12 hours on that, and it was a 3 second fix
Jeremy D. Miller
@jeremydmiller
Oh man, sorry about that. But it happens that way sometimes
Mark Warpool
@CodingGorilla
yea, no problem, it gave me a better understanding about how all the codegen works now :)
I'll submit a PR
Jeremy D. Miller
@jeremydmiller
Cool. Hoping things settle down for me next week so I can start processing OSS things again. Got at least a couple other things to do in both Jasper & Lamar
Jeremy D. Miller
@jeremydmiller
@CodingGorilla I think I’ll have a new Jasper release either tonight or tomorrow
Mark Warpool
@CodingGorilla
Great news, thanks! :+1:
Jeremy D. Miller
@jeremydmiller
It’s not huge, but there’s a Jasper v0.9.13 up today w/ some fixes for @CodingGorilla