search

yglukhov / asyncthreadpool

Awaitable threadpool for nim
MIT License
48 stars 2 forks source link

Chronos support #3

Closed bung87 closed 3 years ago

bung87 commented 3 years ago

PTAL CI passed, macOS install nim fails. https://github.com/bung87/asyncthreadpool/runs/3006960251?check_suite_focus=true

yglukhov commented 3 years ago

CI passed, macOS install nim fails.

It happened to me too. Might be some random issue. Let me know if that's the only issue you have and we can continue with reviewing.

bung87 commented 3 years ago

ok,

  1. I added chronos strict exception check to CI. tp.chanFrom.tryRecv() in dispatchLoop can raise Exception

  2. line 221 in readInto proc , am not sure dont know why call cb(AsyncFD(pipeFd)), chronos cb returns void so I cant do same thing here. 

    when not defined(ChronosAsync):
  if not cb(AsyncFD(pipeFd)):
    addRead(AsyncFD(pipeFd), cb)
else:
  addReader(AsyncFD(pipeFd), cb, data)
return retFuture

  3. in dispatchLoop discard await readInto that can raise.

bung87 commented 3 years ago

wait a minute , I'll rename some variable names. done.

yglukhov commented 3 years ago

So, good to go now? I'm not seeing test results for some reason.

bung87 commented 3 years ago

So, good to go now? I'm not seeing test results for some reason.

yea, only chronos strict exception check fails.

yglukhov commented 3 years ago

yea, only chronos strict exception check fails.

Not sure what that means. The tests run fine for me. I'm merging this.

bung87 commented 3 years ago

see https://github.com/status-im/nim-chronos#exception-effects , if I understand correct in short if you handle exception yourself, chronos will not inject exception check code there.

I do check this for my scorper lib , https://github.com/bung87/scorper/runs/2890536386?check_suite_focus=true
as a library will affected user's project, so I do strictly