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

Prepare for merge #820

Merged
merged 4 commits into from
Sep 11, 2018
Merged

Conversation

larshesel
Copy link
Contributor

A few of the pre-merge items to cross off of the list...

@larshesel
Copy link
Contributor Author

Hey @dergraf , this is interesting. Notice a42f86c and that I reverted it in 8f725d3. Dialyzer correctly complains:

apps/vmq_server/src/vmq_metadata.erl
  32: Call to missing or unexported function vmq_plugin_mgr:disable_plugin/2

As there is no arity two disable_plugin function in vmq_plugin_mgr. But with the fix a test like

rebar3 ct --suite ./apps/vmq_server/test/vmq_clean_session_SUITE.erl --group mqttv4 --case clean_session_qos1_test

Just hangs in the suite teardown which calls vmq_server:stop() - which ends up calling vmq_plugin_mgr:disable_plugin. The test passes when the undefined function is used, but hangs when the correct one is used... got an idea what the problem could be? I reverted the commit with the fix to be able to run the test suites for now

@larshesel
Copy link
Contributor Author

Hey @dergraf I'm done with this - please review. I investigated the dialyzer error about vmq_plugin_mgr:disable_plugin/2 when it's fixed, vmq_plugin_mgr:disable_plugin(vmq_plumtree) simply doesn't return. Maybe you could take a look at that?

@dergraf
Copy link
Contributor

dergraf commented Sep 10, 2018

Analyzed the case. So the root of the problem is a deadlock in the OTP application controller when calling vmq_server:stop().

vmq_server:stop(
    application:stop(vmq_server,
        vmq_server_app:stop(
             vmq_metadata:stop(
                  vmq_plugin_mgr:stop(vmq_plumtree
                      application:stop(vmq_plumtree ===>>>BOOM!

As application:stop is a blocking call, we can't stop another application in the terminate path of an application.

Simple fix would be to run vmq_metadata:stop inside a own process.

@dergraf
Copy link
Contributor

dergraf commented Sep 11, 2018

I've a PR for the issue above in #829
So I'd say revert the related changes here... and merge this PR.

@larshesel larshesel merged commit 7c9996c into vernemq:mqtt5-preview Sep 11, 2018
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.

2 participants