summaryrefslogtreecommitdiff
path: root/vcl/README.lifecycle.md
diff options
context:
space:
mode:
authorHossein <hossein@libreoffice.org>2021-10-26 14:36:34 +0200
committerIlmari Lauhakangas <ilmari.lauhakangas@libreoffice.org>2021-10-26 15:15:05 +0200
commit7e53757f91a176d68cdad1dbde2ecaba1db5c764 (patch)
treeb6e5a2d0d8d5c4ecdf5fc10b1f4ad2505060b518 /vcl/README.lifecycle.md
parente57793158700c48a9a9adf9fb1201e01dc6c270e (diff)
Using .md extension for vcl README files
* Renaming all README files inside 'vcl' to use .md extension * The content was previously converted into Markdown format in dc984e7fed26ed26a2cafecb0c5b27feca56bfe2 Change-Id: If55d363b418ff14864297d02a6986c4fc3625ed7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113531 Tested-by: Ilmari Lauhakangas <ilmari.lauhakangas@libreoffice.org> Reviewed-by: Ilmari Lauhakangas <ilmari.lauhakangas@libreoffice.org>
Diffstat (limited to 'vcl/README.lifecycle.md')
-rw-r--r--vcl/README.lifecycle.md357
1 files changed, 357 insertions, 0 deletions
diff --git a/vcl/README.lifecycle.md b/vcl/README.lifecycle.md
new file mode 100644
index 000000000000..8b0e5de2d0fd
--- /dev/null
+++ b/vcl/README.lifecycle.md
@@ -0,0 +1,357 @@
+# Understanding Transitional VCL Lifecycle
+
+## How it used to look
+
+All VCL classes were explicitly lifecycle managed; so you would do:
+
+```
+Dialog aDialog(...); // old - on stack allocation
+aDialog.Execute(...);
+```
+
+or:
+
+```
+Dialog *pDialog = new Dialog(...); // old - manual heap allocation
+pDialog->Execute(...);
+delete pDialog;
+```
+
+or:
+
+```
+std::shared_ptr<Dialog> xDialog(new pDialog()); // old
+xDialog->Execute(...);
+// depending who shared the ptr this would be freed sometime
+```
+
+In several cases this lead to rather unpleasant code, when
+various `shared_ptr` wrappers were used, the lifecycle was far less than
+obvious. Where controls were wrapped by other ref-counted classes -
+such as UNO interfaces, which were also used by native Window
+pointers, the lifecycle became extremely opaque. In addition VCL had
+significant issues with re-enterancy and event emission - adding
+various means such as DogTags to try to detect destruction of a window
+between calls:
+
+```
+ImplDelData aDogTag( this ); // 'orrible old code
+Show( true, ShowFlags::NoActivate );
+if( !aDogTag.IsDead() ) // did 'this' go invalid yet ?
+ Update();
+```
+
+Unfortunately use of such protection is/was ad-hoc, and far
+from uniform, despite the prevalence of such potential problems.
+
+When a lifecycle problem was hit, typically it would take the
+form of accessing memory that had been freed, and contained garbage due
+to lingering pointers to freed objects.
+
+
+## Where we are now
+
+To fix this situation we now have a `VclPtr` - which is a smart
+reference-counting pointer (`include/vcl/vclptr.hxx`) which is
+designed to look and behave -very- much like a normal pointer
+to reduce code-thrash. `VclPtr` is used to wrap all `OutputDevice`
+derived classes thus:
+
+```
+VclPtr<Dialog> pDialog( new Dialog( ... ), SAL_NO_ACQUIRE );
+...
+pDialog.disposeAndClear();
+```
+
+However - while the `VclPtr` reference count controls the
+lifecycle of the Dialog object, it is necessary to be able to
+break reference count cycles. These are extremely common in
+widget hierarchies as each widget holds (smart) pointers to
+its parents and also its children.
+
+Thus - all previous `delete` calls are replaced with `dispose`
+method calls:
+
+## What is dispose ?
+
+Dispose is defined to be a method that releases all references
+that an object holds - thus allowing their underlying
+resources to be released. However - in this specific case it
+also releases all backing graphical resources. In practical
+terms, all destructor functionality has been moved into
+`dispose` methods, in order to provide a minimal initial
+behavioral change.
+
+As such a `VclPtr` can have three states:
+
+```
+VclPtr<PushButton> pButton;
+...
+assert (pButton == nullptr || !pButton); // null
+assert (pButton && !pButton->isDisposed()); // alive
+assert (pButton && pButton->isDisposed()); // disposed
+```
+
+## `ScopedVclPtr` - making disposes easier
+
+While replacing existing code with new, it can be a bit
+tiresome to have to manually add `disposeAndClear()`
+calls to `VclPtr<>` instances.
+
+Luckily it is easy to avoid that with a `ScopedVclPtr` which
+does this for you when it goes out of scope.
+
+## One extra gotcha - an initial reference-count of 1
+
+In the normal world of love and sanity, eg. creating UNO
+objects, the objects start with a ref-count of zero. Thus
+the first reference is always taken after construction by
+the surrounding smart pointer.
+
+Unfortunately, the existing VCL code is somewhat tortured,
+and does a lot of reference and de-reference action on the
+class -during- construction. This forces us to construct with
+a reference of 1 - and to hand that into the initial smart
+pointer with a `SAL_NO_ACQUIRE`.
+
+To make this easier, we have `Instance` template wrappers
+that make this apparently easier, by constructing the
+pointer for you.
+
+### How does my familiar code change ?
+
+Lets tweak the exemplary code above to fit the new model:
+
+```
+- Dialog aDialog(... dialog params ... );
+- aDialog.Execute(...);
++ ScopedVclPtrInstance<Dialog> pDialog(... dialog params ... );
++ pDialog->Execute(...); // VclPtr behaves much like a pointer
+```
+
+or:
+
+```
+- Dialog *pDialog = new Dialog(... dialog params ...);
++ VclPtrInstance<Dialog> pDialog(... dialog params ...);
+ pDialog->Execute(...);
+- delete pDialog;
++ pDialog.disposeAndClear(); // done manually - replaces a delete
+```
+
+or:
+
+```
+- std::shared_ptr<Dialog> xDialog(new Dialog(...));
++ ScopedVclPtrInstance<Dialog> xDialog(...);
+ xDialog->Execute(...);
++ // depending how shared_ptr was shared perhaps
++ // someone else gets a VclPtr to xDialog
+```
+
+or:
+
+```
+- VirtualDevice aDev;
++ ScopedVclPtrInstance<VirtualDevice> pDev;
+```
+
+Other things that are changed are these:
+
+```
+- pButton = new PushButton(NULL);
++ pButton = VclPtr<PushButton>::Create(nullptr);
+...
+- vcl::Window *pWindow = new PushButton(NULL);
++ VclPtr<vcl::Window> pWindow;
++ pWindow.reset(VclPtr<PushButton>::Create(nullptr));
+```
+
+### Why are these `disposeOnce` calls in destructors?
+
+This is an interim measure while we are migrating, such that
+it is possible to delete an object conventionally and ensure
+that its dispose method gets called. In the 'end' we would
+instead assert that a Window has been disposed in its
+destructor, and elide these calls.
+
+As the object's vtable is altered as we go down the
+destruction process, and we want to call the correct dispose
+methods we need this `disposeOnce();` call for the interim in
+every destructor. This is enforced by a clang plugin.
+
+The plus side of disposeOnce is that the mechanics behind it
+ensure that a `dispose()` method is only called a single time,
+simplifying their implementation.
+
+
+### Who owns & disposes what?
+
+Window sub-classes tend to create their widgets in one of two
+ways and often both.
+
+1. Derive from `VclBuilderContainer`. The `VclBuilder` then owns
+ many of the sub-windows, which are fetched by a `get`
+ method into local variables often in constructors eg.
+
+```
+VclPtr<PushButton> mpButton; // in the class
+, get(mpButton, "buttonName") // in the constructor
+mpButton.clear(); // in dispose.
+```
+
+We only clear, not `disposeAndClear()` in our dispose method
+for this case, since the `VclBuilder` / Container truly owns
+this Window, and needs to dispose its hierarchy in the
+right order - first children then parents.
+
+2. Explicitly allocated Windows. These are often created and
+ managed by custom widgets:
+
+```
+VclPtr<ComplexWidget> mpComplex; // in the class
+, mpComplex( VclPtr<ComplexWidget>::Create( this ) ) // constructor
+mpComplex.disposeAndClear(); // in dispose
+```
+
+ie. an owner has to dispose things they explicitly allocate.
+
+In order to ensure that the VclBuilderConstructor
+sub-classes have their Windows disposed at the correct time
+there is a `disposeBuilder();` method - that should be added
+-only- to the class immediately deriving from
+`VclBuilderContainer`'s dispose.
+
+### What remains to be done?
+
+* Expand the `VclPtr` pattern to many other less
+ than safe VCL types.
+
+* create factory functions for `VclPtr<>` types and privatize
+ their constructors.
+
+* Pass `const VclPtr<> &` instead of pointers everywhere
+ + add `explicit` keywords to VclPtr constructors to
+ accelerate compilation etc.
+
+* Cleanup common existing methods such that they continue to
+ work post-dispose.
+
+* Dispose functions should be audited to:
+ + not leave dangling pointsr
+ + shrink them - some work should incrementally
+ migrate back to destructors.
+
+* `VclBuilder`
+ + ideally should keep a reference to pointers assigned
+ in `get()` calls - to avoid needing explicit `clear`
+ code in destructors.
+
+## FAQ / debugging hints
+
+### Compile with dbgutil
+
+This is by far the best way to turn on debugging and
+assertions that help you find problems. In particular
+there are a few that are really helpful:
+
+```
+vcl/source/window/window.cxx (Window::dispose)
+"Window ( N4sfx27sidebar20SidebarDockingWindowE (Properties))
+ ^^^ class name window title ^^^
+with live children destroyed: N4sfx27sidebar6TabBarE ()
+N4sfx27sidebar4DeckE () 10FixedImage ()"
+```
+
+You can de-mangle these names if you can't read them thus:
+
+```
+$ c++filt -t N4sfx27sidebar20SidebarDockingWindowE
+sfx2::sidebar::SidebarDockingWindow
+```
+
+In the above case - it is clear that the children have not been
+disposed before their parents. As an aside, having a dispose chain
+separate from destructors allows us to emit real type names for
+parents here.
+
+To fix this, we will need to get the dispose ordering right,
+occasionally in the conversion we re-ordered destruction, or
+omitted a `disposeAndClear()` in a `::dispose()` method.
+
+- If you see this, check the order of `disposeAndClear()` in
+ the `sfx2::Sidebar::SidebarDockingWindow::dispose()` method
+
+- also worth `git grep`ing for `new sfx::sidebar::TabBar` to
+ see where those children were added.
+
+### Check what it used to do
+
+While a ton of effort has been put into ensuring that the new
+lifecycle code is the functional equivalent of the old code,
+the code was created by humans. If you identify an area where
+something asserts or crashes here are a few helpful heuristics:
+
+* Read the `git log -u -- path/to/file.cxx`
+
+### Is the order of destruction different?
+
+In the past many things were destructed (in reverse order of
+declaration in the class) without explicit code. Some of these
+may be important to do explicitly at the end of the destructor.
+
+eg. having a `Idle` or `Timer` as a member, may now need an
+ explicit `.Stop()` and/or protection from running on a
+ disposed Window in its callback.
+
+### Is it `clear` not `disposeAndClear`?
+
+sometimes we get this wrong. If the code previously used to
+use `delete pFoo;` it should now read `pFoo->disposeAndClear();`.
+Conversely if it didn't delete it, it should be `clear()` it
+is by far the best to leave disposing to the `VclBuilder` where
+possible.
+
+In simple cases, if we allocate the widget with `VclPtrInstance`
+or `VclPtr<Foo>::Create` - then we need to `disposeAndClear` it too.
+
+### Event / focus / notification ordering
+
+In the old world, a large amount of work was done in the
+`~Window` destructor that is now done in `Window::dispose`.
+
+Since those Windows were in the process of being destroyed
+themselves, their vtables were adjusted to only invoke Window
+methods. In the new world, sub-classed methods such as
+`PreNotify`, `GetFocus`, `LoseFocus` and others are invoked all down
+the inheritance chain from children to parent, during dispose.
+
+The easiest way to fix these is to just ensure that these
+cleanup methods, especially LoseFocus, continue to work even
+on disposed Window sub-class instances.
+
+### It crashes with some invalid memory...
+
+Assuming that the invalid memory is a Window sub-class itself,
+then almost certainly there is some cockup in the
+reference-counting; eg. if you hit an `OutputDevice::release`
+assert on `mnRefCount` - then almost certainly you have a
+Window that has already been destroyed. This can easily
+happen via this sort of pattern:
+
+```
+Dialog *pDlg = VclPtr<Dialog>(nullptr /* parent */);
+// by here the pDlg quite probably points to free'd memory...
+```
+
+It is necessary in these cases to ensure that the `*pDlg` is
+a `VclPtr<Dialog>` instead.
+
+### It crashes with some invalid memory #2...
+
+Often a `::dispose` method will free some `pImpl` member, but
+not `NULL` it; and (cf. above) we can now get various `virtual`
+methods called post-dispose; so:
+
+a) `delete pImpl; pImpl = NULL; // in the destructor`
+b) `if (pImpl && ...) // in the subsequently called method`