Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any ideas why this doesn't exit ? Possibly worker-thread related ? #46

Closed
markddrake opened this issue Sep 22, 2022 · 7 comments
Closed

Comments

@markddrake
Copy link

markddrake commented Sep 22, 2022

Given this


import wtf             from 'wtfnode'

import YadamuCLI       from '../../node/cli/yadamuCLI.js'
import YadamuLibrary   from '../../node/lib/yadamuLibrary.js'

class Test extends YadamuCLI {}

async function main() {
  
  try {
    const yadamuTest = new Test();
    try {
      await yadamuTest.doTests();
    } catch (e) {
	  yadamuTest.reportError(e)
    }
    await yadamuTest.close();
  } catch (e) {
	YadamuLibrary.reportError(e)
  }
  
  wtf.dump()

}

main()

export { Test as default}

With one set of actions my code exits as expected and the dump() shows the following on the way out the door.

C:\Development\WTF-YADAMU>node src\qa\cli\wtfTest.js config=qa\YDB2\t-imp.json
2022-09-22T16:20:58.479Z [QA][Environemnt][x64][win32][v18.9.0]: Running tests
2022-09-22T16:21:02.542Z [INFO][READER][YABASC][3.0][Manager]: Ready.
2022-09-22T16:21:02.549Z [INFO][WRITER][Teradata][17.10.02.01][DATA_ONLY][Manager]: Ready.
...
2022-09-22T16:21:08.331Z [QA][YADAMU][REGRESSION][qa\YDB2\t-imp.json]: Errors: 0. Warnings: 0. Failed: 0. Elapsed Time: 00:00:09.853s.
[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 0 (tty)
    - Listeners:
Unable to determine callsite for "onkeypress". Did you require `wtfnode` at the top of your entry point?
      - keypress: onkeypress @ unknown:0
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Others:
  - MessagePort

C:\Development\WTF-YADAMU>node src\qa\cli\wtfTest.js config=qa\YDB2\t-exp.json

With a different set of actions the node process hangs and I have to CTRL-C. In this case dump() shows:

C:\Development\YADAMU>node C:\Development\YADAMU\src\qa\cli\test.js CONFIG=qa\ydb2\exp1.json

2022-09-22T15:54:39.511Z [QA][Environemnt][x64][win32][v18.9.0]: Running tests
2022-09-22T15:54:43.830Z [INFO][READER][Teradata][17.10.02.01][Manager]: Ready.
....
2022-09-22T16:21:53.502Z [QA][YADAMU][REGRESSION][qa\YDB2\t-exp.json]: Errors: 0. Warnings: 0. Failed: 0. Elapsed Time: 00:00:33.040s.
[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 0 (tty)
    - Listeners:
Unable to determine callsite for "onkeypress". Did you require `wtfnode` at the top of your entry point?
      - keypress: onkeypress @ unknown:0
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
^C
C:\Development\WTF-YADAMU>

I only see these problems when running against teradata. If I run against a different database, eg mysql, the output from dump() at the end of process is identical, but the process exists normally.

The major difference between Yadamu's teradata interface and its other database interfaces, is that the teradata interface makes use of 'worker-threads' to work around the lack of async IO support in teradata's npm module. Is it possible that wtfnode is not reporting something related to worker-threads ?

@markddrake
Copy link
Author

It was an issue with a connection left open in the worker thread.

@myndzi
Copy link
Owner

myndzi commented Sep 29, 2022

Hey there -- thanks for following up. I saw the original post but then got busy and neglected to respond; I apologize!

As you surmised, wtfnode can't observe worker threads unless it's explicitly added to their code. To get reporting output from child processes, you'd need to add the "require" to the child process's code, so that it can hook the things it needs to, and I'm not 100% sure about the signal passing to trigger a report at that point tbh.

It would perhaps be possible to hook child_process and inject wtfnode into other node processes, but that starts to get really problematic; there's a lot of api surface area to cover, and it may not be readily possible to determine if the new process is, in fact, something we can hook. My gut feeling is that pretending to support that workflow will give a false sense of correctness and hit tons of edge cases that don't work right. Perhaps it would be better just to call out this caveat strongly with the readme?

@markddrake
Copy link
Author

markddrake commented Sep 29, 2022

Could wtf at least point out there is a worker that is causing the main to fail to exit, without necessarily identifying what the problem with the worker thread is? That way you would know to go ahead and add wtf to the worker. I didn't actually try adding wtf to the worker in my case as I spotted that I had failed to close a database connection manually. My main was calling worker.unref() when it had finished with the worker and getting a clean return and I wasn't sure where I would add a wtf.dump() inside the worker.

@myndzi
Copy link
Owner

myndzi commented Sep 29, 2022

wtfnode should already list child processes, but if you're manually calling unref then I don't think it will show up in process._getActiveHandles() (the source from which everything is derived). But in that case it also shouldn't keep the parent process awake, so I'm not entirely sure what the deal is. I'd need some code I could use to reproduce to really explore, I think. Most of the work that went into wtfnode was munging/hacking/reverse engineering node internals, it's difficult to just "come up with" an answer to a new scenario without the ability to play with it :)

Any chance you can derive a minimal reproduction case that models your problem?

@markddrake
Copy link
Author

markddrake commented Sep 29, 2022

Kris

The words "Minimal" and "Teradata" rarely if ever occur in the same sentence ... :)

My project Yadamu currently supports around 10 databases, and Teradata is the only one where this occurred, since it was the only one that required me to use Worker threads.

Setting up the testcase for Teradata involves creating a Frankenstein like environment which requires a Docker Image that runs Oracle Virtual Box, that then manages to load a teradata VM that was originally designed for use with VMWARE, Yes, I freely admit to being a Masochist when it comes to databases, but I wouldn't wish trying to set that environment up on my worst enemy, and I can only get it to run in an environment where docker is running on ubuntu 20.04.

I have a couple of deadlines I am working on but once they are over, I will see if I can come up with a contrived example that might show this without requiring a database.

@myndzi
Copy link
Owner

myndzi commented Sep 29, 2022

😂 I feel for you! It seems like you've identified the key points, so hopefully something can be done. Sorry that wtfnode turned out to be more of a hassle than a help in this instance :(

@markddrake
Copy link
Author

Don't worry about it, it has saved me many hours of debugging overall. Thank you

@myndzi myndzi closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants