jamulussoftware / jamulussoftware/jamulus

Jamulus iOS crashes on Qt 5.15.19, likely due to stack allocation in main.cpp

Open
#3,808 13 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C
Stars
1.1k
Forks
248
Avg merge
2d 3h
Merged PRs (30d)
9

Description

**Describe the bug**

Jamulus iOS crashed on my local Qt 5.15.19 build. I was fighting to debug it and then asked GPT 5 SOL. It was extremely fast (and expensive) and pointed out that we allocate too much on the stack causing corruption leading to overwrite something in Qt. I think it does indeed make sense to change this.
Especially the Client object is likely too large.

Also we should remove src/iOS/ios_app_delegate.mm/.h probably as it pollutes some functions in Qt.

Unfortunately, I could not follow the thinking output and cleared the session too quickly - the crash got fixed.

Suggestion: we should use std::unique_ptr's and heap allocation. Maybe this also fixes the Qt6 crashes.

The produced fix actually works.
Proposed diff:
```
diff --git a/src/main.cpp b/src/main.cpp
index a518c2e5..1250d7bf 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -980,17 +980,18 @@ int main ( int argc, char** argv )
#ifndef SERVER_ONLY
if ( bIsClient )
{
- CClient Client ( iPortNumber, iQosNumber, bNoAutoJackConnect, strClientName, bDisableIPv6, bMuteMeInPersonalMix );
+ std::unique_ptr Client (
+ new CClient ( iPortNumber, iQosNumber, bNoAutoJackConnect, strClientName, bDisableIPv6, bMuteMeInPersonalMix ) );

// Create Settings with the client pointer
- CClientSettings Settings ( &Client, strIniFileName );
+ CClientSettings Settings ( Client.get(), strIniFileName );
Settings.Load ( CommandLineOptions );
- Client.SetSettings ( &Settings );
+ Client->SetSettings ( &Settings );

# ifndef NO_JSON_RPC
if ( pRpcServer )
{
- new CClientRpc ( &Client, &Settings, pRpcServer, pRpcServer );
+ new CClientRpc ( Client.get(), &Settings, pRpcServer, pRpcServer );
}
# endif

@@ -1006,7 +1007,7 @@ int main ( int argc, char** argv )

// GUI object
CClientDlg
- ClientDlg ( &Client, &Settings, strConnOnStartupAddress, bShowComplRegConnList, bShowAnalyzerConsole, bMuteStream, nullptr );
+ ClientDlg ( Client.get(), &Settings, strConnOnStartupAddress, bShowComplRegConnList, bShowAnalyzerConsole, bMuteStream, nullptr );

// show dialog
ClientDlg.show();
@@ -1018,8 +1019,8 @@ int main ( int argc, char** argv )
{
if ( !strConnOnStartupAddress.isEmpty() )
{
- Client.SetServerAddr ( strConnOnStartupAddress );
- Client.Start();
+ Client->SetServerAddr ( strConnOnStartupAddress );
+ Client->Start();
}

// only start application without using the GUI
@@ -1033,32 +1034,32 @@ int main ( int argc, char** argv )
{
// Server:
// actual server object
- CServer Server ( iNumServerChannels,
- strLoggingFileName,
- strServerBindIP4,
- strServerBindIP6,
- iPortNumber,
- iQosNumber,
- strDirectoryAddress,
- strServerListFileName,
- strServerInfo,
- strServerPublicIP,
- strServerListFilter,
- strWelcomeMessage,
- strRecordingDirName,
- bDisconnectAllClientsOnQuit,
- bUseDoubleSystemFrameSize,
- bDisableRaw,
- bUseMultithreading,
- bDisableRecording,
- bDelayPan,
- bDisableIPv6,
- eLicenceType );
+ std::unique_ptr Server ( new CServer ( iNumServerChannels,
+ strLoggingFileName,
+ strServerBindIP4,
+ strServerBindIP6,
+ iPortNumber,
+ iQosNumber,
+ strDirectoryAddress,
+ strServerListFileName,
+ strServerInfo,
+ strServerPublicIP,
+ strServerListFilter,
+ strWelcomeMessage,
+ strRecordingDirName,
+ bDisconnectAllClientsOnQuit,
+ bUseDoubleSystemFrameSize,
+ bDisableRaw,
+ bUseMultithreading,
+ bDisableRecording,
+ bDelayPan,
+ bDisableIPv6,
+ eLicenceType ) );

#ifndef NO_JSON_RPC
if ( pRpcServer )
{
- new CServerRpc ( &Server, pRpcServer, pRpcServer );
+ new CServerRpc ( Server.get(), pRpcServer, pRpcServer );
}

#endif
@@ -1066,7 +1067,7 @@ int main ( int argc, char** argv )
if ( bUseGUI )
{
// load settings from init-file (command line options override)
- CServerSettings Settings ( &Server, strIniFileName );
+ CServerSettings Settings ( Server.get(), strIniFileName );
Settings.Load ( CommandLineOptions );

// load translation
@@ -1076,7 +1077,7 @@ int main ( int argc, char** argv )
}

// GUI object for the server
- CServerDlg ServerDlg ( &Server, &Settings, bStartMinimized, nullptr );
+ CServerDlg ServerDlg ( Server.get(), &Settings, bStartMinimized, nullptr );

// show dialog (if not the minimized flag is set)
if ( !bStartMinimized )
@@ -1096,7 +1097,7 @@ int main ( int argc, char** argv )
// strDirectoryAddress is wanted
if ( !strDirectoryAddress.isEmpty() )
{
- Server.SetDirectoryType ( AT_CUSTOM );
+ Server->SetDirectoryType ( AT_CUSTOM );
}

pApp->exec();
```
**To Reproduce**

Compile Qt 5.15.19 on macOS, build Jamulus.
**Expected behavior**

Jamulus does not crash
**Screenshots**

/
**Operating system**

**Version of Jamulus**

**Additional context**

Contributor guide

Open the contributing guide

Research direction

Start in src/main.cpp around the client and server startup paths, then reproduce the crash with a Qt 5.15.19 macOS build. Review the proposed heap-allocation change and the referenced src/iOS/ios_app_delegate.mm/.h files. Done means the iOS build no longer crashes and the affected startup paths still work.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, ios
Domain
audio-video-rtc, mobile-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.