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

add SetUnhandledExceptionFilter support to win32 so that crash dump c… #80

Open
wants to merge 1 commit into
base: koffi2
Choose a base branch
from

Conversation

longhun12346
Copy link

@longhun12346 longhun12346 commented Apr 28, 2024

add SetUnhandledExceptionFilter support to win32 so that crash dump can work as expected, to fix Koromix/koffi#155

@longhun12346
Copy link
Author

@Koromix Hi, please take some time look at this. I need to make my app whole as soon as possiable.Thanks.

@longhun12346
Copy link
Author

@Koromix Hi, please take some time look at this. I need to make my app whole as soon as possiable.Thanks.

@Koromix Hi, have you check this ? Is there any bug in this pr ?

@Koromix
Copy link
Owner

Koromix commented May 23, 2024

Sorry, I don't have much time to work on this right now. Your patch looks good but will have a performance impact, and I'd prefer to find a way to make the SEH unwinder across the Koffi "assembly wall", if possible, so that it can cross the Koffi stack change and frames and properly go up to the default handler.

In the mean time, I encourage you to include your alternative build of koffi.node in your package, especially if it is for a single platform (Win32). The binary file koffi.node is the only file actually needed to use Koffi. All other file in the NPM package are optional (source code for rebuild, test files, etc.). You can use something like require('./patched_koffi.node').

You don't need to load Koffi through the package Koffi index.js entry point. All the JS code does is make some basic checks and implement deprecated functions.

@longhun12346
Copy link
Author

longhun12346 commented May 25, 2024

Sorry, I don't have much time to work on this right now. Your patch looks good but will have a performance impact, and I'd prefer to find a way to make the SEH unwinder across the Koffi "assembly wall", if possible, so that it can cross the Koffi stack change and frames and properly go up to the default handler.

In the mean time, I encourage you to include your alternative build of koffi.node in your package, especially if it is for a single platform (Win32). The binary file koffi.node is the only file actually needed to use Koffi. All other file in the NPM package are optional (source code for rebuild, test files, etc.). You can use something like require('./patched_koffi.node').

You don't need to load Koffi through the package Koffi index.js entry point. All the JS code does is make some basic checks and implement deprecated functions.

@Koromix Hi, thanks for reply. I know waht you mean, but i think it's not possible to across the Koffi "assembly wall", when dispatch exception windows check the every SEH link list node’s address is between stackbase and stacklimit or not , if we link the exceptionlist back to default stack, the check will fail and windows didn't dispatch exception to our handler(try...catch block). The only way i can find for now is modify stackbase and stacklimit to include both the default stack and koffi's stack area, but since windows may have used them anywhere so it's too dangerous. Or do you have a way to cheat windows on this?

@Koromix Koromix force-pushed the koffi2 branch 9 times, most recently from 75dcc40 to 52ebc2c Compare June 17, 2024 11:56
@Koromix Koromix force-pushed the koffi2 branch 5 times, most recently from b8199ea to 3926b91 Compare July 11, 2024 11:23
@Koromix Koromix force-pushed the koffi2 branch 7 times, most recently from e4be51a to 9a234eb Compare November 15, 2024 11:48
@Koromix Koromix force-pushed the koffi2 branch 3 times, most recently from 1c5c4b7 to 058f3fb Compare November 26, 2024 11:02
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

Successfully merging this pull request may close these issues.

2 participants