Bug 439676 - Crash on startup when providing two directories as args
Summary: Crash on startup when providing two directories as args
Status: RESOLVED FIXED
Alias: None
Product: kdiff3
Classification: Applications
Component: application (other bugs)
Version First Reported In: unspecified
Platform: Compiled Sources All
: NOR grave
Target Milestone: ---
Assignee: michael
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-09 11:46 UTC by Flav
Modified: 2021-07-10 22:29 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In: 1.9.3
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Flav 2021-07-09 11:46:36 UTC
Application: kdiff3 (1.9.70(64 bits))

Qt Version: 5.15.2
Frameworks Version: 5.83.0
Operating System: Linux 5.12.10-arch1-1 x86_64
Windowing System: X11
Drkonqi Version: 5.22.0
Distribution: "Arch Linux"

-- Information about the crash:
With The master branch, when launching with two arguments, it segfaults.

This fixes the bug :

```
diff --git a/src/pdiff.cpp b/src/pdiff.cpp
index 8467ffc..79c2753 100644
--- a/src/pdiff.cpp
+++ b/src/pdiff.cpp
@@ -1610,6 +1610,10 @@ bool KDiff3App::doDirectoryCompare(const bool bCreateNewInstance)
         m_pMainWidget->hide();
         setUpdatesEnabled(true);
 
+        if(m_dirinfo == nullptr){
+          m_dirinfo = QSharedPointer<DirectoryInfo>::create();
+          MergeFileInfos::setDirectoryInfo(m_dirinfo);
+        }
         (*m_dirinfo) = DirectoryInfo(f1, f2, f3, destDir);
 
         bool bSuccess = m_pDirectoryMergeWindow->init(
```

But I Think m_dirinfo should have been already initialized... Because this is redundant with `void KDiff3App::slotFileOpen()`.


However, I may ask why is MergeFileInfos::m_dirinfo static ? I have the feeling it's not a good design, and that it would have been better to make it an instance attribute...

The crash can be reproduced every time.

-- Backtrace:
Application: KDiff3 (kdiff3), signal: Segmentation fault

[KCrash Handler]
#4  0x00005594b2afb786 in QScopedPointer<FileAccessJobHandler, QScopedPointerDeleter<FileAccessJobHandler> >::reset (this=0x8, other=0x5594b3062ba0) at /usr/include/qt/QtCore/qscopedpointer.h:155
#5  0x00005594b2af5e18 in FileAccess::operator= (this=0x0, b=...) at /home/leo/projects/sources/kdiff3/src/fileaccess.cpp:165
#6  0x00005594b2a88943 in DirectoryInfo::operator= (this=0x0) at /home/leo/projects/sources/kdiff3/src/DirectoryInfo.h:13
#7  0x00005594b2a834e3 in KDiff3App::doDirectoryCompare (this=0x5594b30a17f0, bCreateNewInstance=false) at /home/leo/projects/sources/kdiff3/src/pdiff.cpp:1613
#8  0x00005594b2a3a4b4 in KDiff3App::completeInit (this=0x5594b30a17f0, fn1=..., fn2=..., fn3=...) at /home/leo/projects/sources/kdiff3/src/kdiff3.cpp:414
#9  0x00005594b2a2f658 in KDiff3Shell::KDiff3Shell (this=0x5594b2e51840, bCompleteInit=true, __in_chrg=<optimized out>, __vtt_parm=<optimized out>) at /home/leo/projects/sources/kdiff3/src/kdiff3_shell.cpp:60
#10 0x00005594b2a2c181 in main (argc=3, argv=0x7ffcdb3a39b8) at /home/leo/projects/sources/kdiff3/src/main.cpp:186
[Inferior 1 (process 35053) detached]

Possible duplicates by query: bug 430698, bug 429420, bug 410860.

Rapporter à https://bugs.kde.org/
Comment 1 michael 2021-07-10 19:47:36 UTC
m_dirinfo is the same for all instances of MergeFileInfos and therefor uses some unneeded overhead as an instance variable. However a better solution is actually to make it global. That will be done in master along with the fix for this crash. Looks it should be possible to back port it 1.9. In any event the crash itself will be fixed in both.
Comment 2 michael 2021-07-10 22:28:38 UTC
Git commit a0fad8add09df00f722c97506d7f73b483d60464 by Michael Reeves.
Committed on 10/07/2021 at 21:28.
Pushed by mreeves into branch 'master'.

Move DirectoryInfo to global scope.

This makes the code cleaner and less confusing we only need one DirectoryInfo.
Keep shared pointer status for now maybe reviewed later but is out of scope.
KDiff3 has multiple code paths for starting a compare so the old intialization did not cover all cases.
Move this first time setup to the default iniialization.
This is safe for most code. Constuctors for other global/static variables however can not depend on this having taken place.
FIXED-IN:1.9.3

M  +3    -0    src/DirectoryInfo.cpp
M  +2    -0    src/DirectoryInfo.h
M  +6    -8    src/MergeFileInfos.cpp
M  +1    -4    src/MergeFileInfos.h
M  +0    -2    src/kdiff3.h
M  +5    -11   src/pdiff.cpp

https://invent.kde.org/sdk/kdiff3/commit/a0fad8add09df00f722c97506d7f73b483d60464
Comment 3 michael 2021-07-10 22:29:15 UTC
Git commit 7f9461c54707bc3261fbf02c877d03717ac2ee68 by Michael Reeves.
Committed on 10/07/2021 at 22:26.
Pushed by mreeves into branch '1.9'.

Move DirectoryInfo to global scope.

This makes the code cleaner and less confusing we only need one DirectoryInfo.
Keep shared pointer status for now maybe reviewed later but is out of scope.
KDiff3 has multiple code paths for starting a compare so the old intialization did not cover all cases.
Move this first time setup to the default iniialization.
This is safe for most code. Constuctors for other global/static variables however can not depend on this having taken place.
FIXED-IN:1.9.3

M  +1    -0    src/CMakeLists.txt
A  +40   -0    src/DirectoryInfo.cpp     [License: GPL(v2.0+)]
M  +7    -28   src/DirectoryInfo.h
M  +6    -9    src/MergeFileInfos.cpp
M  +1    -4    src/MergeFileInfos.h
M  +9    -15   src/directorymergewindow.cpp
M  +0    -2    src/kdiff3.h
M  +10   -9    src/pdiff.cpp

https://invent.kde.org/sdk/kdiff3/commit/7f9461c54707bc3261fbf02c877d03717ac2ee68