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

DAEMON-336 - If lCallbacks is empty don't call the fnCallbacks to avoid an ACCESS_VIOLATION #32

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

Conversation

stevehof
Copy link

This is a more specific patch instead of removing the apxHandleManagerDestroy(); as suggested as a workaround. It seems that if there are no callbacks, this is empty and can throw an ACCESS VIOLATION. Instead ignore the callbacks if lCallbacks is empty.

Comment on lines +500 to +501
/* Call the user callback first */
(*hObject->fnCallback)(hObject, WM_CLOSE, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this inside the test of lCallbacks?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also throw an access violation, i believe in the JVM.dll which is with a version i do not have pdb files for. I tested this on master with the current 1.2.5 fixes and this could also likely be caused by code from the java side of things. I do not have a good understanding of the issues here but i do know that lCallbacks was empty and that fnCallback would also raise an access violation. What are these meant to be calling? I presume something in the java, however i could not find any documentation, and this is the only "issue" to an otherwise well running service.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my comment in DAEMON-336, I'd like to see the steps to reproduce this so I can dig into what is going on before applying any patches.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevehof ping?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only adding the if is probably OK. Something like:
if (lpCall->fnCallback)
(*lpCall->fnCallback)(hObject, WM_CLOSE, 0, 0);
There are a bunch of place where we test the callback before calling it...

@garydgregory
Copy link
Member

Any luck reproducing this issue or including changes for the upcoming RC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants