search

mrahhal / MR.AspNetCore.Jobs

A background processing library for Asp.Net Core.
MIT License
59 stars 9 forks source link

Jobs does not dispose DI objects #4

Closed BrandweerAA closed 7 years ago

BrandweerAA commented 7 years ago

Hi!

First of all: great library! Thanks for sharing.

It seems when scheduling and executing services with DI, the injected services are not released after successful execution of that job. In this case, it's a DbContext object that is not references anywhere else and direct invocation (without Jobs) releases this context after execution.

Is this by design? If so, is there any way to override this behaviour?

mrahhal commented 7 years ago

Hi,

This isn't the intended behavior no. When a job executes, a scoped provider gets created, which is supposed to be disposed when the job finishes execution. I'll investigate soon. Which version are you using?

And a small note, this library is very close to a "1.0" release. But as I'm not having enough time lately for reviewing it, I'm a bit reluctant about releasing a stable version yet. I'd be very happy if you can provide any info/feedback, thanks!

BrandweerAA commented 7 years ago

Hi mrahhal,

I was using the nget package, so version 0.6.0. Meanwhile, I've used the current sources with the same behaviour. .NET Core version is 1.1.2 stable, targetting .NET Framework 4.6.1.

It's strange because the code in DelayedJobProcessor looks good on first inspection but I must admit I've not had much experience with DI internals yet.

Re your reluctance: I've not been using the cron functionality as of yet but apart from that I've not had any difficulties and my impression is that it is pretty stable otherwise!

Edit: it seems the service is destroyed ok, as is the injected repository. The DbContext injected in the repository however, is not. This context is only created by the repository and only once. I've even marked the context as transient but that does not help either.

mrahhal commented 7 years ago

Yes I reviewed the processor too, I'm pretty sure I'm correctly disposing the scoped context. I did a small test and looks like it's true. I may have an idea of what's causing the problem. I'll look into it.

Unrelated: Please be advised that starting from next version, the internal migrations table I'm using will change to match the options in SqlServerOptions. So if you want to maintain compatibility and you can't delete the old table you'll have to configure SqlServerOptions to match your currently used table.

mrahhal commented 7 years ago

Problem was registering JobFactory with singleton lifetime, incorrectly. It should've been scoped. Fixed now, thanks.

If you can't wait for a package release you can use the ci packages.

BrandweerAA commented 7 years ago

Ah, missed that one. Working as it should now. Thank you!