DevWiki/calculator
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix #563: Odd dependency cycle (#570)

Browse Source 
There is an odd dependency in CalcEngine.

`CalculatorManager` inherits `ICalcDisplay` and implements a set of virtual calls it exposes, in particular `SetPrimaryDisplay`.
2517854836/src/CalcManager/Header%20Files/ICalcDisplay.h (L13)

When setting a mode in `CalculatorManager`, e.g. 2517854836/src/CalcManager/CalculatorManager.cpp (L208)
`this` (here: an instance of `CalculatorManager`) gets passed as an argument to the newly created `CCalcEngine` as `ICalcDisplay` pointer and the engine is stored as `unique_ptr` member field of `CalculatorManager`.

In the destructor of `CalculatorManager`, a single function is called, `MemorizedNumberClearAll` which then invokes `ProcessCommand(IDC_MCLEAR)` on current engine, gets passed on to `CCalcEngine::ProcessCommandWorker`, to `DisplayNum`, to `CCalcEngine::SetPrimaryDisplay` and finally to `m_pCalcDisplay->SetPrimaryDisplay`, but here `m_pCalcDisplay` _was_ the instance of `CalculatorManager` that just got its destructor called.

2517854836/src/CalcManager/CalculatorManager.cpp (L46)
2517854836/src/CalcManager/CalculatorManager.cpp (L475)
2517854836/src/CalcManager/CEngine/scicomm.cpp (L87)
2517854836/src/CalcManager/CEngine/scicomm.cpp (L133)
2517854836/src/CalcManager/CEngine/scidisp.cpp (L124)
2517854836/src/CalcManager/CEngine/scicomm.cpp (L837)

It will likely differ by implementation on how exactly, but the [standard suggests](http://eel.is/c++draft/class.cdtor#4) that will invoke the pure virtual `ICalcDisplay::SetPrimaryDisplay`. In case of GCC I believe the vtable is already destroyed by the time you enter dtor's body.

There appears to be no reason to call `MemorizedNumberClearAll` in `CalculatorManager::~CalculatorManager()` because the calc manager and all its engines are going down anyway.  Therefore, removing the call (and thus, the destructor which would then be empty).


Fixes #563: Odd dependency cycle
This commit is contained in:
Michał Janiszewski 2019-07-26 08:24:12 +02:00 committed by Howard Wolosky
parent 0722781fc6
commit 60c5c39ee5
2 changed files with 0 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/CalcManager/CalculatorManager.cpp
View File

@ -37,15 +37,6 @@ namespace CalculationManager
CCalcEngine::InitialOneTimeOnlySetup(*m_resourceProvider); CCalcEngine::InitialOneTimeOnlySetup(*m_resourceProvider);
} }
/// <summary>
/// Destructor for CalculatorManager
/// Ends two CCalcEngine
/// </summary>
CalculatorManager::~CalculatorManager()
{
this->MemorizedNumberClearAll();
}
/// <summary> /// <summary>
/// Call the callback function using passed in IDisplayHelper. /// Call the callback function using passed in IDisplayHelper.
/// Used to set the primary display value on ViewModel /// Used to set the primary display value on ViewModel

1
src/CalcManager/CalculatorManager.h
View File

@ -103,7 +103,6 @@ namespace CalculationManager
void MemoryItemChanged(unsigned int indexOfMemory) override; void MemoryItemChanged(unsigned int indexOfMemory) override;
CalculatorManager(ICalcDisplay* displayCallback, IResourceProvider* resourceProvider); CalculatorManager(ICalcDisplay* displayCallback, IResourceProvider* resourceProvider);
~CalculatorManager();
void Reset(bool clearMemory = true); void Reset(bool clearMemory = true);
void SetStandardMode(); void SetStandardMode();