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

after fb_shutdown(), end-of-process cleanup should not do anything [CORE4475] #4795

Closed
firebird-automations opened this issue Jun 28, 2014 · 10 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: Lionel Elie Mamane (lmamane)

LibreOffice embeds FireBird, telling it to use a subdirectory of the LibreOffice tempdir for the firebird lockdir.

At process shutdown, ~Cleanup from file init.cpp is called, which calls this stack:
allClean()
Firebird::InstanceControl::destructors()
Firebird::InstanceControl::InstanceList::destructors()
Firebird::InstanceControl::InstanceLink<jrd::StorageInstance, ...>::dtor
jrd::StorageInstance::dtor
~ConfigStorage
ISC_remove_map_file(sh_mem const*)
ISC_remove_map_file(char const*)
gds_prefix_lock
os_utils::createLockDirectory(char cont*)

However, the LibreOffice temporary directory, which is the parent of the firebird lockdir, has been deleted by then, which causes the mkdir() to fail, throwing an exception that eventually causes (in development build) firebird to call abort().

So we changed LibreOffice to call fb_shutdown(0, 1) when it does not need firebird anymore, expecting that then ~Cleanup would not be called, or become a noop or ... We checked that fb_shutdown is actually called, and returns success. However, ~Cleanup still tries to do work, for that to create a lockdir, which fails and abort().

Please make fb_shutdown() make all the cleanups so that nothing is done at process exit() time anymore.

Commits: cb65a2a f1e3ba6 FirebirdSQL/fbt-repository@fc0251c FirebirdSQL/fbt-repository@0030ebb

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

assignee: Alexander Peshkov [ alexpeshkoff ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

It's rather problematic to do nothing in ~Cleanup - it's a place in code where dtors of global objects are executed, and this should be done at exit time or when dynamic library is unloaded. Certainly attempts to create files/directories at this moment are absolutely wrong - and this is what I've fixed here.

I do not have access to Mac and therefore to test the fix compiled it on linux using set of system calls used/present on Mac. Everything looks fine but if you have any problems on real build - feel free to contact.

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 2.5.4 [ 10585 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: Lionel Elie Mamane (lmamane)

Great, thanks. I'm forwarding to one of our Mac guys, and will come back to you with a confirmation.

@firebird-automations
Copy link
Collaborator Author

Commented by: Lionel Elie Mamane (lmamane)

Tested OK on Mac.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pcisar

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Closed [ 6 ] => Closed [ 6 ]

QA Status: No test

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Closed [ 6 ] => Closed [ 6 ]

QA Status: No test => Cannot be tested

@firebird-automations
Copy link
Collaborator Author

Commented by: Lionel Elie Mamane (lmamane)

In principle, it can be tested:
* use a _debug_ build of Firebird
* start a firebird instance, opening a database, do some work
* call fb_shutdown(0, 1)
* rm -rf /path/to/fb_lockdir
* exit(0) the process or unload the firebird dynamic library

If the process aborts, the test failed. If it exits cleanly, the test succeeded.

@firebird-automations
Copy link
Collaborator Author

Commented by: @dyemanov

Hard to do with our automated CI framework. We don't test debug builds and we don't test embedded alone. Also, I doubt we can call fb_shutdown() from Python. The comment was about adding a test to the regression suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants