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

Fix PROJ usage accross fork() calls (fixes #2221) #2222

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

rouault
Copy link
Member

@rouault rouault commented Feb 5, 2020

No description provided.

@rouault rouault added this to the 3.0.5 milestone Feb 6, 2020
@rouault rouault merged commit 8034590 into OSGeo:master Feb 6, 2020
@TimoRoth
Copy link

TimoRoth commented Feb 7, 2020

I don't actually think this is enough. As per the SQlite Website (https://www.sqlite.org/howtocorrupt.html#_carrying_an_open_database_connection_across_a_fork_):

Do not even call sqlite3_close() on a database connection from a child process if the connection was opened in the parent. It is safe to close the underlying file descriptor, but the sqlite3_close() interface might invoke cleanup activities that will delete content out from under the parent, leading to errors and perhaps even database corruption.

I'm not even sure if this can be sanely handled from within gdal, it might need some logic in proj itself, maybe using pthread_atfork(), if that call is possible to use?
Also makes me wonder why sqlite, being this sensitive to this, does not have a fork handler on its own.

Right now, the only two ways I see to do this is to either set all DB handles to NULL in the child processes, and just accept that they leak.
Or right before forking, which can be caught via above atfork(), close and destroy all of them. Which could lead to funny side effects with threading.
Would also need tracking of all open contexts. In a somehow thread safe manner.

Another way is of course to just forward the warning from sqlite, to not use objects across fork() calls.
Which seems reasonable given the alternatives, but is terribly annoying for Python users using multiprocessing.

@rouault
Copy link
Member Author

rouault commented Feb 7, 2020

I'm not even sure if this can be sanely handled from within gdal

Presumably the above warning from sqlite3 doc is in situations where the database might be modified, but here it is read-only. At least the test I added in the fix consistently failed before the fix, and consistently works afterwards.

it might need some logic in proj itself, maybe using pthread_atfork(), if that call is possible to use?

That would be annoying to do. I'd suggest the pragmatic way to test the fix and see if it works before going into further complications.

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