Error: 'invoke' is not GC-safe as it performs an indirect call here

[theAkito]

winim/winim/com.nim

Lines 1000 to 1002 in b7b3260

	if self.disp.Invoke(dispid, &IID_NULL, LOCALE_USER_DEFAULT, invokeType, &dp, &ret, &excep, nil).ERR:
	if cast[pointer](excep.pfnDeferredFillIn).notNil:
	{.gcsafe.}: discard excep.pfnDeferredFillIn(&excep)

I had trouble with GC safety in my application and the root cause was located in the place seen above.
The actual execution is marked as {.gcsafe.}, however the "indirect execution" to find out, if the value isNil, is not.

My quick fix for this issue is the following:

  if self.disp.Invoke(dispid, &IID_NULL, LOCALE_USER_DEFAULT, invokeType, &dp, &ret, &excep, nil).ERR:
    var excepinfoPfnDeferredFillInIsNotNil: bool
    {.gcsafe.}: excepinfoPfnDeferredFillInIsNotNil = cast[pointer](excep.pfnDeferredFillIn).notNil
    if excepinfoPfnDeferredFillInIsNotNil:
      {.gcsafe.}: discard excep.pfnDeferredFillIn(&excep)

Not sure, if this is a good solution, so I wanted to open an Issue and get feedback, before opening any Pull Request.

[khchen]

Can you give me some code to test?

[theAkito]

@khchen

import
  urlly,
  strutils,
  winim/com
from streams import
  newFileStream,
  writeData,
  close
from zippy import
  dfGzip,
  uncompress

when true:
  proc fetch*(req: Request, filepath = ""): Response {.discardable, gcsafe.} =
    result = Response()
    let
      reqObj = CreateObject("WinHttp.WinHttpRequest.5.1")
      isFile = filepath.len > 0
    if req.verb.len == 0: req.verb = "GET"
    try:
      reqObj.open(req.verb.toUpperAscii(), $req.url)
      for header in req.headers:
        reqObj.setRequestHeader(header.key, header.value)
      if req.body.len > 0:
        reqObj.send(req.body)
      else:
        reqObj.send()
    except:
      result.error = getCurrentExceptionMsg()
    result.url = req.url
    if result.error.len != 0: return
    result.code = parseInt(reqObj.status)
    block text:
      if isFile: break text
      let bodyIsEmpty =
        try: reqObj.responseBody == VT_EMPTY
        except: false
      if bodyIsEmpty: break text
      result.body = string(fromVariant[COMBinary](reqObj.responseBody))
    block file:
      if not isFile: break file
      var
        unknown = fromVariant[ptr IUnknown](reqObj.responseStream)
        pStream: ptr IStream
      if not unknown.QueryInterface(&IID_IStream, cast[ptr pointer](pStream.addr)).SUCCEEDED: return
      defer: pStream.Release()
      var
        buffer = newString(1024)
        cbRead: ULONG
        bodyFileStream = newFileStream(filepath, fmWrite)
      if bodyFileStream.isNil: return
      defer: bodyFileStream.close()
      while true:
        if not pStream.Read(&buffer, ULONG buffer.len, cbRead.addr).SUCCEEDED: return
        if cbRead == 0: break ## No more bytes read -> we have finished reading.
        let remainingbuffer = buffer[0..<cbRead]
        bodyFileStream.writeData(remainingbuffer.cstring, remainingbuffer.len)
    let headers = $reqObj.getAllResponseHeaders()
    for headerLine in headers.split(winLineEnd):
      let arr = headerLine.split(":", 1)
      if arr.len != 2: continue
      result.headers[arr[0].strip()] = arr[1].strip()
    if result.headers["content-encoding"].toLowerAscii() == "gzip":
      result.body = uncompress(result.body, dfGzip)

This will never compile with --threads:on, because get is not gcsafe, because invoke is not gcsafe, because cast[pointer](excep.pfnDeferredFillIn).notNil is not gcsafe.

[khchen]

I think you found a regression bug of the nim compiler. Nim compiler 1.2.x don't have this problem. And the line that compiler complain about is surely gcsafe.

A quick workaround is change proc notNil to template notNil (in fact, it should be a template). This change that resolves the problem proves a compiler bug.

Or just mark these two lines as {.gcsafe.}.

    {.gcsafe.}:
      if cast[pointer](excep.pfnDeferredFillIn).notNil:
        discard excep.pfnDeferredFillIn(&excep)

[theAkito]

Or just mark these two lines as {.gcsafe.}.

Somehow, no matter how I tried to use it like that, it never worked with more than a single line.
If I added a second line, or more, then it always errored out.

[khchen]

Fixed.

[TechThupport]

@khchen sorry to message here but getting the same errors for another function when building with --threads:on

winim-3.9.0/winim/clr.nim(80, 6) Warning: 'toObject' is not GC-safe as it calls 'getRuntimeHelp' [GcUnsafe2]
winim-3.9.0/winim/clr.nim(289, 6) Warning: 'invoke' is not GC-safe as it calls 'toObject' [GcUnsafe2]
test.nim(87, 6) Error: 'testProc' is not GC-safe as it calls 'invoke'