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

Bug inside ordering of objects in configure() #6

Closed
thuotdwz opened this issue May 19, 2020 · 4 comments · Fixed by #11
Closed

Bug inside ordering of objects in configure() #6

thuotdwz opened this issue May 19, 2020 · 4 comments · Fixed by #11
Labels
enhancement New feature or request

Comments

@thuotdwz
Copy link
Contributor

Hi, I have noticed another bug. If inside configure the order of initialization is done so queues driver is before database, like this:

app.queues.use(.fluent(.sqlite))
app.databases.use(.sqlite(.memory))

Application will then receive an error when running migrations:

ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.
ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.

If order is changed into:

app.databases.use(.sqlite(.memory))
app.queues.use(.fluent(.sqlite))

Then the application will run successfully.

I see there is a note in the code

    // How do we report that something goes wrong here? Since makeQueue cannot throw.
    let dialect = (db as? SQLDatabase)?.dialect.name ?? "unknown"
    let dbType = QueuesFluentDbType(rawValue: dialect) ?? .none

Maybe here we can log a warning message using context.logger to let the developer know the initialization order is wrong so they can fix it?

This would be helpful to much more easily tack down the issue. It took me a while to figure out!

Thanks for working on this package, it is useful!

@m-barthelemy
Copy link
Owner

@thuotdwz I'll look into it, but first of all please do note that for now this driver won't work with Sqlite (as stated in the README). It's Postgres, Mysql or MariaDB only.

@m-barthelemy m-barthelemy added the enhancement New feature or request label May 19, 2020
@m-barthelemy
Copy link
Owner

Unfortunately I was unable to reproduce.
If I call app.queues.use(.fluent(.sqlite)) before app.databases.use(.sqlite(.memory)), I get Fatal error: No datatabase configuration registered for DatabaseID(string: "sqlite").: file which is pretty explicit despite the typo :-)

@thuotdwz
Copy link
Contributor Author

Thanks for your reply, could be there is something else in my app causing the shut down event loop issue. Appreciate the readme note!

Also I think SQLITE can be made to work with a small change, I will file a new ticket for thaat.

@thuotdwz
Copy link
Contributor Author

I found out the problem with ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.

It turns out, we cannot shutdown eventLoopGroup in FluentQueuesDriver shutdown, because this object is not owned by queues driver.

thuotdwz added a commit to thuotdwz/vapor-queues-fluent-driver that referenced this issue May 20, 2020
This will fix error message:

ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.

when using Ctrl+C to shut down application. EventLoopGroup here is not owned by FluentQueuesDriver and used by other objects during shutdown procedure

Fixes m-barthelemy#6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants