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

Cygwin: system sqlite3 modifies DLL search order #30157

Closed
embray opened this issue Jul 16, 2020 · 15 comments
Closed

Cygwin: system sqlite3 modifies DLL search order #30157

embray opened this issue Jul 16, 2020 · 15 comments

Comments

@embray
Copy link
Contributor

embray commented Jul 16, 2020

Under normal operation Windows searches for DLLs in the following order:

  1. In the same directory as the main executable.
  2. In various standard system directories.
  3. Search $PATH.

On Cygwin, most DLLs are stored under /usr/bin, which is inserted early on the $PATH, but when in the Sage environment $SAGE_LOCAL/bin supersedes it.

However, I discovered that the Cygwin port of sqlite3 contains the following nasty patch, for reasons I can't be sure of:

@@ -47710,13 +48357,52 @@ SQLITE_API int sqlite3_os_init(void){
   assert( winSysInfo.dwAllocationGranularity>0 );
   assert( winSysInfo.dwPageSize>0 );

+#ifdef _WIN32
+  module = osGetModuleHandleW(L"CYGWIN1.DLL");
+  if( !module ){
+    module = osGetModuleHandleW(L"MSYS-2.0.DLL");
+  }
+  if( !module ){
+    module = osGetModuleHandleW(L"MSYS-1.0.DLL");
+  }
+  if( module ){
+    for( i=81; i<ArraySize(aSyscall); ++i ){
+        aSyscall[i].pCurrent = (SYSCALL) osGetProcAddressA(module,
+            aSyscall[i].zName);
+    }
+  }
+#endif
+
+#if SQLITE_OS_UNIX
+  sqlite3_os_unix_init();
+#endif
+
   sqlite3_vfs_register(&winVfs, 1);

+#if !defined(SQLITE_OMIT_LOAD_EXTENSION)
+  if( cygwin_conv_path ){
+    WCHAR buf[MAX_PATH];
+    cygwin_conv_path(CCP_POSIX_TO_WIN_W, "/usr/bin",
+        buf, MAX_PATH*sizeof(WCHAR));
+    osSetDllDirectoryW(buf);
+#ifdef _WIN32
+  }else if( cygwin_conv_to_full_win32_path ){
+    WCHAR buf[MAX_PATH];
+    char *buf1 = (char *)buf;
+    int i = MAX_PATH;
+    cygwin_conv_to_full_win32_path("/usr/bin", buf1);
+    while(--i>=0) buf[i] = buf1[i];
+    osSetDllDirectoryW(buf);
+#endif
+  }
+#endif
+
 #if defined(SQLITE_WIN32_HAS_WIDE)
   sqlite3_vfs_register(&winLongPathVfs, 0);
 #endif

The function SetDllDirectoryW is a system function which works a little like LD_LIBRARY_PATH or LD_PRELOAD allowing an application to insert a non-standard directory into the DLL search path.

Why sqlite3 is doing this I don't know. It has something to do with loading extensions apparently, but there's no reason it should assume I always want to load extensions from /usr/bin (e.g. what if I'm using a custom build of sqlite3 installed in /usr/local/bin.

It does this also when the library is first initialized, as opposed to just doing it before loading an extension and then unsetting it. Thus this change to DLL loading behavior affects the rest of the application for the lifetime of the application. It does get undone if sqlite3_shutdown() is called but this never happens normally.

How does this affect Sage? It's not even obvious that Sage uses sqlite3, but in fact IPython does to store its history, so a sqlite3 database is connected to when starting up the Sage/IPython REPL. This in turn impacts DLL search order for all libraries that haven't been loaded yet, and can cause Cygwin's system versions of those libraries to be privileged over any copies in Sage.

I think this might actually explain some related bugs I've seen in the past, but I'm not sure.

Component: porting: Cygwin

Author: Erik Bray

Branch: 628e7d0

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/30157

@embray embray added this to the sage-9.2 milestone Jul 16, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 16, 2020

comment:1

sqlite3 is a dependency of python3. Not sure if it is only linked when the corresponding module is used, or in general

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

comment:2

Only when the corresponding module is used.

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Commit: 628e7d0

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Author: Erik Bray

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

comment:3

It's not pretty, but it does fix the issue. Open to better ideas for what to name this and/or where to insert the fix.


New commits:

628e7d0Trac #30157: Undo sqlite3's munging of DLL search paths

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Branch: u/embray/ticket-30157

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 16, 2020

comment:4

Perhaps this should also be reported as a bug to the cygwin maintainers?

@dimpase
Copy link
Member

dimpase commented Jul 17, 2020

comment:5

Isn't Sage using sqlite3 in src/sage/graphs/graph_database.py ?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 22, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 25, 2020

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Jul 28, 2020

Changed branch from u/embray/ticket-30157 to 628e7d0

@embray
Copy link
Contributor Author

embray commented Jul 29, 2020

Changed commit from 628e7d0 to none

@embray
Copy link
Contributor Author

embray commented Jul 29, 2020

comment:10

Replying to @mkoeppe:

Perhaps this should also be reported as a bug to the cygwin maintainers?

I did, the maintainer of the Cygwin package for sqlite3 agreed it's a bug and we discussed a possible fix: https://cygwin.com/pipermail/cygwin/2020-July/245536.html

@embray
Copy link
Contributor Author

embray commented Jul 29, 2020

comment:11

Replying to @dimpase:

Isn't Sage using sqlite3 in src/sage/graphs/graph_database.py ?

It's actually use during Sage start-up, but only when starting in the IPython REPL. It's used by IPython because it stores the interactive history in a sqlite database.

This is actually how I was able to narrow this down: because the issue didn't occur when I imported sage in a normal Python session, or ran something like sage -c 'print(1 + 1)'. It would only happen when starting with the interactive prompt, which was very bizarre.

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

4 participants