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

Misc fixes for undefined behavior, crashes, and build failure #3465

Merged
merged 7 commits into from
Feb 6, 2025

Conversation

cebtenzzre
Copy link
Member

@cebtenzzre cebtenzzre commented Feb 5, 2025

The theme of this PR is things I noticed while trying to make Windows ARM more stable. None of them seem to affect the official releases in practice.

  • AddressSanitizer detected a stack-use-after-return in database.cpp, which is now fixed.
  • The backend no longer crashes if one of the search dirs does not exist (related to cmake: do not modify gpt4all.app after signing it #3417)
  • Builds against the debug version of Qt, which is the default on Windows debug builds, were complaining about a call to setParent from the wrong thread. This is fixed.
  • The Visual Studio build (not used in CI) was failing due to a duplicate string in gpt4all_ro_RO.ts, which has been fixed.
  • Debug builds were crashing when inconsequential network requests were failing, such as Mixpanel. This is now reduced to a warning when the QNetworkReply reports an error (such as "Connection refused").
  • We were hitting a QRingBuffer assertion because we were using the log file from multiple threads without a mutex.

`info` is local to scanQueue, so we have to store a copy in the
DocumentReader to extend its lifetime. Not sure how this even worked
before.

Signed-off-by: Jared Van Bortel <[email protected]>
Attempting to notify the objects of this change from a different thread
causes an error in debug builds:

```
ASSERT failure in QCoreApplication::sendEvent: "Cannot send events to
objects owned by a a different thread. Current thread 0x0x1a9dc360490.
Receiver '' (of type 'ChatModel') was created in thread
0x0x1a9ad1a99f0", file
C:\Users\qt\work\qtbase\src\corelib\kernel\qcoreapplication.cpp, line
557
```

QObject::setParent is not documented to be thread-safe.

Signed-off-by: Jared Van Bortel <[email protected]>
This entry causes the Visual Studio build to fail.

Signed-off-by: Jared Van Bortel <[email protected]>
Signed-off-by: Jared Van Bortel <[email protected]>
@cebtenzzre cebtenzzre marked this pull request as ready for review February 5, 2025 16:46
This fixes the following assertion failure in Qt debug builds:
```
ASSERT: "bytes <= bufferSize" in file C:\Users\qt\work\qt\qtbase\src\corelib\tools\qringbuffer.cpp, line 75
```

The message handler is called from the thread that sends the message, so
we need to make sure only one thread can access the file at a time,
since QFile is not inherently threadsafe.

Signed-off-by: Jared Van Bortel <[email protected]>
@cebtenzzre cebtenzzre merged commit 22ebd42 into main Feb 6, 2025
4 of 18 checks passed
@cebtenzzre cebtenzzre deleted the jared/misc-fixes-feb5 branch February 10, 2025 16:45
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