#75 destroy hooks

owlkettle.nimble

@@ -18,7 +18,7 @@ task examples, "Build examples":
   withDir "examples":
     for file in findExamples("."):
       echo "INFO: Compile " & file
-      exec "nim c --hints:off --verbosity:0 " & file
+      exec "nim c --hints:off --path:.. --verbosity:0 " & file
       echo "INFO: OK"
       echo "================================================================================"
 

owlkettle/customPragmas.nim

@@ -0,0 +1,8 @@
+## Definess a custom pragma called "locker".
+## This does nothing when compiled with a nim version 2.0 or higher,
+## but applies the "locks: 0" pragma if compiled for e.g. 1.6.X.
+## "locks: 0" is applied to ensure that this code never accesses locked data in user-defined procs.
+when NimMajor >= 2:
+  {.pragma: locker.}
+else:
+  {.pragma: locker, locks: 0.}

owlkettle/dataentries.nim

@@ -25,6 +25,8 @@ when defined(nimPreviewSlimSystem):
   import std/formatfloat
 import widgets, widgetdef, guidsl
 
+include ./customPragmas
+
 when defined(owlkettleDocs) and isMainModule:
   echo "# Dataentries Widgets\n\n"
 
@@ -145,19 +147,19 @@ method parse(entry: FormulaEntryState, text: string): (bool, float64) =
       tokens: seq[Token]
       cur: int
 
-  proc add(stream: var TokenStream, token: Token) {.locks: 0.} =
+  proc add(stream: var TokenStream, token: Token) {.locker.} =
     stream.tokens.add(token)
 
-  proc next(stream: TokenStream, kind: TokenKind): bool {.locks: 0.} =
+  proc next(stream: TokenStream, kind: TokenKind): bool {.locker.} =
     result = stream.cur < stream.tokens.len and
              stream.tokens[stream.cur].kind == kind
 
-  proc take(stream: var TokenStream, kind: TokenKind): bool {.locks: 0.} =
+  proc take(stream: var TokenStream, kind: TokenKind): bool {.locker.} =
     result = stream.next(kind)
     if result:
       stream.cur += 1
 
-  proc tokenize(text: string): TokenStream {.locks: 0.} =
+  proc tokenize(text: string): TokenStream {.locker.} =
     const
       WHITESPACE = {' ', '\n', '\r', '\t'}
       OP = {'+', '-', '*', '/', '^', '%'}
@@ -191,7 +193,7 @@ method parse(entry: FormulaEntryState, text: string): (bool, float64) =
             kind = TokenNumber
           result.add(Token(kind: kind, value: name))
 
-  proc eval(stream: var TokenStream, level: int): tuple[valid: bool, val: float64] {.locks: 0.} =
+  proc eval(stream: var TokenStream, level: int): tuple[valid: bool, val: float64] {.locker.} =
     var prefix = 1.0
     if stream.take(TokenPrefixOp) and stream.tokens[stream.cur - 1].value == "-":
       prefix = -1.0

owlkettle/gtk.nim

@@ -340,6 +340,7 @@ proc g_signal_connect_data*(widget: pointer,
 proc g_type_from_name*(name: cstring): GType
 proc g_object_new*(typ: GType): pointer {.varargs.}
 proc g_object_ref*(obj: pointer)
+proc g_object_weak_ref*(obj: pointer, notify: proc (data, oldObjectPtr: pointer) {.cdecl.}, data: pointer)
 proc g_object_unref*(obj: pointer)
 proc g_object_set_property*(obj: pointer, name: cstring, value: ptr GValue)
 proc g_type_fundamental*(id: GType): GType

owlkettle/mainloop.nim

@@ -22,9 +22,36 @@
 
 import gtk, widgetdef
 
-type Stylesheet* = ref object
-  provider: GtkCssProvider
-  priority: int
+type 
+  StylesheetObj = object
+    provider: GtkCssProvider
+    priority: int
+
+  Stylesheet* = ref StylesheetObj
+
+when(NimMajor >= 2):
+  proc `=destroy`(stylesheet: StylesheetObj) =
+    if isNil(stylesheet.provider):
+      return
+
+    g_object_unref(pointer(stylesheet.provider))
+else:
+  proc `=destroy`(stylesheet: var StylesheetObj) =
+    if isNil(stylesheet.provider):
+      return
+
+    g_object_unref(pointer(stylesheet.provider))
+proc `=copy`*(dest: var StylesheetObj, source: StylesheetObj) =
+  let areSameObject = pointer(source.provider) == pointer(dest.provider)
+  if areSameObject:
+    return
+
+  `=destroy`(dest)
+  wasMoved(dest)
+  if not isNil(source.provider):
+    g_object_ref(pointer(source.provider))
+
+  dest.provider = source.provider
 
 const DEFAULT_PRIORITY = 600
 

owlkettle/widgets.nim

@@ -26,7 +26,7 @@ import std/[unicode, sets, tables, options, asyncfutures, hashes, times]
 when defined(nimPreviewSlimSystem):
   import std/assertions
 import gtk, widgetdef, cairo, widgetutils
-
+include ./customPragmas
 when defined(owlkettleDocs) and isMainModule:
   echo "# Widgets"
 
@@ -458,18 +458,48 @@ type
   Colorspace* = enum
     ColorspaceRgb
 
-  Pixbuf* = ref object
+  # Wrapper for the GdkPixbuf pointer to work with destructors of nim's ARC/ORC
+  # Todo: As of 16.09.2023 it is mildly buggy to try and to `PixBuf = distinct GdkPixbuf` and
+  # have destructors act on that new type directly. It was doable as shown in Ticket #75 and Github PR #81
+  # But required a =sink hook for no understandable reason *and* seemed risky due to bugs likely making it unstable.
+  # The pointer wrapped by intermediate type used as ref-type via an alias approach seems more stable for now.
+  # Re-evaluate this in Match 2024 to see whether we can remove the wrapper obj.
+  PixbufObj* = object
     gdk: GdkPixbuf
+
+  Pixbuf* = ref PixbufObj
 
-proc finalizer(pixbuf: Pixbuf) =
-  g_object_unref(pointer(pixbuf.gdk))
+when(NimMajor >= 2):
+  proc `=destroy`(pixbuf: PixbufObj) =
+    if isNil(pixbuf.gdk):
+      return
+
+    g_object_unref(pointer(pixbuf.gdk))
+else:
+  proc `=destroy`(pixbuf: var PixbufObj) =
+    if isNil(pixbuf.gdk):
+      return
+
+    g_object_unref(pointer(pixbuf.gdk))
 
+proc `=copy`*(dest: var PixbufObj, source: PixbufObj) =
+  let areSameObject = pointer(source.gdk) == pointer(dest.gdk)
+  if areSameObject:
+    return
+
+  `=destroy`(dest)
+  wasMoved(dest)
+  if not isNil(source.gdk):
+    g_object_ref(pointer(source.gdk))
+
+  dest.gdk = source.gdk
+
 proc newPixbuf(gdk: GdkPixbuf): Pixbuf =
   if gdk.isNil:
     raise newException(ValueError, "Unable to create Pixbuf from GdkPixbuf(nil)")
-  new(result, finalizer=finalizer)
+  result = Pixbuf()
   result.gdk = gdk
-
+  
 proc newPixbuf*(width, height: int,
                 bitsPerSample: int = 8,
                 hasAlpha: bool = false,
@@ -549,7 +579,7 @@ proc handlePixbufReady(stream: pointer, result: GAsyncResult, data: pointer) {.c
   var error = GError(nil)
   let pixbuf = gdk_pixbuf_new_from_stream_finish(result, error.addr)
   if error.isNil:
-    future.complete(Pixbuf(gdk: pixbuf))
+    future.complete(newPixbuf(pixbuf))
   else:
     let message = $error[].message
     future.fail(newException(IoError, "Unable to load pixbuf: " & message))
@@ -793,7 +823,7 @@ proc `valIcon=`*(button: Button, name: string) =
 proc updateChild*(state: Renderable,
                   child: var BoxChild[WidgetState],
                   updater: BoxChild[Widget],
-                  setChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                  setChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   if updater.widget.isNil:
     if not child.widget.isNil:
       child.widget = nil
@@ -1105,9 +1135,9 @@ type PanedChild[T] = object
 proc buildPanedChild(child: PanedChild[Widget],
                      app: Viewable,
                      internalWidget: GtkWidget,
-                     setChild: proc(paned, child: GtkWidget) {.cdecl, locks: 0.},
-                     setResize: proc(paned: GtkWidget, val: cbool) {.cdecl, locks: 0.},
-                     setShrink: proc(paned: GtkWidget, val: cbool) {.cdecl, locks: 0.}): PanedChild[WidgetState] =
+                     setChild: proc(paned, child: GtkWidget) {.cdecl, locker.},
+                     setResize: proc(paned: GtkWidget, val: cbool) {.cdecl, locker.},
+                     setShrink: proc(paned: GtkWidget, val: cbool) {.cdecl, locker.}): PanedChild[WidgetState] =
   child.widget.assignApp(app)
   result = PanedChild[WidgetState](
     widget: child.widget.build(),
@@ -1986,6 +2016,12 @@ type
     underline*: Option[UnderlineKind]
     style*: Option[CairoFontSlant]
 
+  # Wrapper for the GtkTextBuffer pointer to work with destructors of nim's ARC/ORC
+  # Todo: As of 16.09.2023 it is mildly buggy to try and to `TextBuffer = distinct GtkTextBuffer` and
+  # have destructors act on that new type directly. It was doable as shown in Ticket #75 and Github PR #81
+  # But required a =sink hook for no understandable reason *and* seemed risky due to bugs likely making it unstable.
+  # The pointer wrapped by intermediate type used as ref-type via an alias approach seems more stable for now.
+  # Re-evaluate this in Match 2024 to see whether we can remove the wrapper obj.
   TextBufferObj = object
     gtk: GtkTextBuffer
 
@@ -1995,11 +2031,15 @@ type
   TextTag* = GtkTextTag
   TextSlice* = HSlice[TextIter, TextIter]
 
-proc finalizer(buffer: TextBuffer) =
-  g_object_unref(pointer(buffer.gtk))
+when(NimMajor >= 2):
+  proc `=destroy`(buffer: TextBufferObj) =
+    g_object_unref(pointer(buffer.gtk))
+else:
+  proc `=destroy`(buffer: var TextBufferObj) =
+    g_object_unref(pointer(buffer.gtk))
 
 proc newTextBuffer*(): TextBuffer =
-  new(result, finalizer=finalizer)
+  new(result)
   result.gtk = gtk_text_buffer_new(nil.GtkTextTagTable)
 
 {.push hint[Name]: off.}
@@ -2839,7 +2879,7 @@ renderable ContextMenu:
 
   hooks menu:
     (build, update):
-      proc replace(box, oldMenu, newMenu: GtkWidget) {.locks: 0.} =
+      proc replace(box, oldMenu, newMenu: GtkWidget) {.locker.} =
         if not oldMenu.isNil:
           gtk_widget_remove_controller(box, state.controller)
           state.controller = GtkEventController(nil)

owlkettle/widgetutils.nim

@@ -24,6 +24,7 @@
 
 import std/[sets]
 import gtk, widgetdef
+include ./customPragmas
 
 proc redraw*[T](event: EventObj[T]) =
   if event.app.isNil:
@@ -74,7 +75,7 @@ proc updateStyle*[State, Widget](state: State, widget: Widget) {.inline.} =
 proc updateChild*(state: Renderable,
                   child: var WidgetState,
                   updater: Widget,
-                  replaceChild: proc(widget, oldChild, newChild: GtkWidget) {.locks: 0.}) =
+                  replaceChild: proc(widget, oldChild, newChild: GtkWidget) {.locker.}) =
   if updater.isNil:
     if not child.isNil:
       replaceChild(state.internalWidget, unwrapInternalWidget(child), nil.GtkWidget)
@@ -92,8 +93,8 @@ proc updateChild*(state: Renderable,
 proc updateChild*(state: Renderable,
                   child: var WidgetState,
                   updater: Widget,
-                  addChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.},
-                  removeChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                  addChild: proc(widget, child: GtkWidget) {.cdecl, locker.},
+                  removeChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   proc replace(widget, oldChild, newChild: GtkWidget) =
     if not oldChild.isNil:
       removeChild(widget, oldChild)
@@ -105,7 +106,7 @@ proc updateChild*(state: Renderable,
 proc updateChild*(state: Renderable,
                   child: var WidgetState,
                   updater: Widget,
-                  setChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                  setChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   proc replace(widget, oldChild, newChild: GtkWidget) =
     setChild(widget, newChild)
 
@@ -114,8 +115,8 @@ proc updateChild*(state: Renderable,
 proc updateChildren*(state: Renderable,
                      children: var seq[WidgetState],
                      updates: seq[Widget],
-                     addChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.},
-                     removeChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                     addChild: proc(widget, child: GtkWidget) {.cdecl, locker.},
+                     removeChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   updates.assignApp(state.app)
 
   var
@@ -151,9 +152,9 @@ proc updateChildren*(state: Renderable,
 proc updateChildren*(state: Renderable,
                      children: var seq[WidgetState],
                      updates: seq[Widget],
-                     addChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.},
-                     insertChild: proc(widget, child: GtkWidget, index: cint) {.cdecl, locks: 0.},
-                     removeChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                     addChild: proc(widget, child: GtkWidget) {.cdecl, locker.},
+                     insertChild: proc(widget, child: GtkWidget, index: cint) {.cdecl, locker.},
+                     removeChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   updates.assignApp(state.app)
 
   var it = 0
@@ -196,8 +197,8 @@ proc toGtk*(align: Align): GtkAlign = GtkAlign(ord(align))
 proc updateAlignedChildren*(state: Renderable,
                             children: var seq[AlignedChild[WidgetState]],
                             updates: seq[AlignedChild[Widget]],
-                            addChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.},
-                            removeChild: proc(widget, child: GtkWidget) {.cdecl, locks: 0.}) =
+                            addChild: proc(widget, child: GtkWidget) {.cdecl, locker.},
+                            removeChild: proc(widget, child: GtkWidget) {.cdecl, locker.}) =
   updates.assignApp(state.app)
   var
     it = 0