-
Notifications
You must be signed in to change notification settings - Fork 581
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
OpenSIPS 1.8.4 can not be started when there are dialogs in the dialogs table #197
Comments
Hello, I've just tried to replicate this - all went fine for me.
Best Regards, |
I'll have a look right now for the logs. But: |
You can find the log here: Not much interesting to see over there. I'm surprised you can not reproduce it because it is 100% reproducable here. |
Vlad, Could you reproduce it ? Something different between your test and mine is that we are using profiles (set_dlg_profile). That might be the culprit. I seem to remember a few commits on opensips dealing with profiles loading on restart (but I'm not sure). |
Hello, Still cannot reproduce it ( tested with multiple legs and profiles ). Are you using OpenSIPS 1.8 from git, or from the 1.8.4 tar on the website ? Would it be possible to give me access to your server so I can do further debugging there ? Best Regards, |
Hi, Would an OpenVPN access be ok ? |
Btw, it is 1.8.4 from the tarball. We have a few patches (which are all available as pull requests), but located in the presence modules. |
Hello, After further talks on IRC, the crash is related to using dialog profiles with cachedb_* persistency. Best Regards, |
As discussed on IRC, and for the record, the crash is the following:
|
This is due to the dialog module mod_init method using the cachedb module methods before child_init has been called. In that case, the SQL handled is still set to NULL. Fixes issue OpenSIPS#197.
Vlad, I have pushed a pull request for master. |
Hi Vlad, Do you have a patch to test ? Indeed, my "workaround" is unsafe. Thank you, Le 16/04/14 11:54, vladpaiu a écrit :
Damien SANDRAS Ekiga Project |
Hi, What is weird is that you commented out instructions that were not present in the original cachedb_sql implementation we provided to you. In other words, the 1.8 backtrace I sent you corresponds to a crash without cdb_dbf.close(cdb_db_handle); Are you sure the correct bug is fixed ? |
And what I do not understand either is that Bogdan closed my pull request that was moving the cdb_db_handle = cdb_dbf.init(&db_url) from child_init to mod_init telling that it was not correct because we are using a global connection in that case, but your fix does the same except it does not remove the initialisation from child_init. That means that we start the module with a global connection, then move to a "per process" connection as soon as possible ? Is that intended ? |
Hello, In my tests, the crash no longer occurs with the provided commit. The underlying issue is that, indeed, the cachedb_sql module does not properly implement the cachedb interface - it manages it's own global connection to the back-end instead of allowing the interface to do it ( this is a side effect of the fact that the module was implemented following the cachedb_local template where there is no actual connection ). For a full example of the way the interface was meant to work, please take a look at the cachedb_redis ( eg. the init function from the cachedb interface should actually create the connection to the backend , instead of creating the connection in the mod_init func of the back-end module. Further on, that connection can be retrieved when running a command by accessing the cachedb_con->data pointer ). Best Regards, |
Hi, Understood. I hope it is a safe fix :) |
This did not happen with 1.8.3.
Reproducing the bug is easy:
The text was updated successfully, but these errors were encountered: