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

Python3 crashes on importing sqlite3 - solution included #62

Closed
pshem opened this issue Feb 28, 2019 · 7 comments
Closed

Python3 crashes on importing sqlite3 - solution included #62

pshem opened this issue Feb 28, 2019 · 7 comments

Comments

@pshem
Copy link

pshem commented Feb 28, 2019

When using the python3x image, one of the big no-gos is import sqlite3. Because it is a standard library module, it is present in every python3x image. However, importing it directly or indirectly causes OSv to crash:
selectionshot_2019-02-07_13 32 15

I needed sqlite to work after finding out django would insist on importing sqlite even when using a different database as the main data storage. I struck gold when I realized that OSv-apps contained a working sqlite3 application, already compiled as a shared library sqlite.so. So I decided to see if they would be compatible... and they were! Here are the steps I took:

  1. Update sqlite ot the newest version by changing parameters in sqlite's GET to
    VERSION=3270200 and YEAR=2019,
  2. Build an sqlite image
  3. Build a python3x,httpserver image
  4. Upload /apps/sqlite/sqlite.so to OSv with the httpserver, placing it as /usr/lib/libsqlite3.so.0
  5. Import sqlite on python3 successfully.

I was wondering what the best way to upstream this python improvement? For personal use, I added
rsync -a $BASEDIR/../sqlite/sqlite.so $ROOTFS/usr/lib/libsqlite3.so.0 at the end of main() in python3x's GET, which works just fine. For upstream use, we probably shouldn't assume sqlite has been build before python was. I tried using api.require in module.py but it only seems to accept a single argument...

I would be happy to write a PR fixing sqlite in python3x, but I need some help figuring out the proper way to do this. In the meantime, I'd be happy to upstream the version update to sqlite since that seems to work just fine, but I've only tried a few functions from the sqlite shell outside using it in a python project, so I'm not confident it is a non-breaking update.

@pshem pshem changed the title Python3 crashes on importing sqlite Python3 crashes on importing sqlite3 Feb 28, 2019
@pshem pshem changed the title Python3 crashes on importing sqlite3 Python3 crashes on importing sqlite3 - solution included Feb 28, 2019
@nyh
Copy link

nyh commented Feb 28, 2019

See also this (very old) issue on the missing mremap() needed by sqlite: cloudius-systems/osv#184

@pshem
Copy link
Author

pshem commented Feb 28, 2019

@nyh I've seen it. As nice as it would be to get a more optimized version of OSv, I don't have the time to get back into C++ this quarter and have to rely on things that are implemented. My current priority is to reduce the number of OSv patches my application is carrying

@wkozaczuk
Copy link
Collaborator

wkozaczuk commented Feb 28, 2019 via email

@nyh
Copy link

nyh commented Mar 3, 2019

@pshem, some answers to your questions/ideas:

  1. api.require() indeed takes just one parameter but you can call it multiple times. You can see an example in apps/erlang/module.py.
  2. However, adding to python3 a require('sqlite') will mean that python3 will always include sqlite. Do we want that? If not, you don't need a require, you can make it optional - i.e., python3's Makefile (or GET) could check whether ../sqlite was compiled, and if so use it, and if not, not. This will allow someone to do "scripts/build image=sqlite,python3" and get python3 and sqlite. Of course, nobody except you will remember that this specific combination has a special meaning :-)
  3. In general, yes, we should update the versions on all the applications to a recent stable/good version. If this bothers someone, let them complain, but I doubt it will.

@pshem
Copy link
Author

pshem commented Mar 3, 2019

  1. I assume that calling api.require() triggers a build and appends the required module's usr.manifest without adding the module's command line to the built image. However, because sqlite would be added to the built image before python, python would overwrite it. This needs to be avoided, but having a guarantee that sqlite has been built helps a lot

  2. The current python3 always needs sqlite. Currently a version using MREMAP is copied form the OS and breaks OSv when actually used. There are 3 solutions possible:

    • strip sqlilte(the python interpreter no longer has all elements of the standard library),
    • add support for MREMAP(best option, most work, makes things just work),
    • swap in a working sqlite(probably using api.require, I can write that patch)
  3. In that case, I'll make a PR updating sqlite3. I'm hoping reviewing my PRs isn't too much of a drag ;)

@nyh
Copy link

nyh commented Mar 3, 2019

  1. Indeed.
  2. If python3 always needs sqlite then indeed it can api.require() sqlite, and we can blacklist sqlite as a library we don't want to automatically copy from the host. We should probably have a clear comment about the place in the code doing this, referring to issue 184, because we'll probably want to undo this (and just take the host's normal sqlite) if we ever fix the mremap issue.

pshem added a commit to pshem/osv-apps that referenced this issue Mar 4, 2019
by using sqlite compiled without `MREMAP`(see cloudius-systems/osv#184 and cloudius-systems#62)
@pshem
Copy link
Author

pshem commented Mar 4, 2019

#64 is a patch using api.require() to make sure sqlite is built and overwriting the broken libsqlite.so.0 from the host. The commit has notes on which changes can be reverted upon implementing MREMAP. Would you rather continue the discussion here or under the PR?

@pshem pshem closed this as completed Mar 11, 2019
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

No branches or pull requests

3 participants