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

Refreshing security log throws exception if it is already refreshing #159

Open
UltimateEvil opened this issue Apr 2, 2023 · 5 comments
Open

Comments

@UltimateEvil
Copy link

Click on security log and spam the refresh a bit.

"Exception of type 'System.ExecutionEngineException' was thrown."

Specifically in

StartHandlingSecurityLogEvents(true)
...
                dataView = CollectionViewSource.GetDefaultView(EventsReader.Entries);
                gridLog.ItemsSource = dataView;

Upon changing to

                dataView = CollectionViewSource.GetDefaultView(EventsReader.Entries);
                gridLog.ItemsSource = null;
                gridLog.ItemsSource = dataView;

I instead got an exception
System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

  • it understandably caused issues in other parts of the code which depended in it being initialised.

The issue seems to only happen before the numebr of new entries gets updated

@wokhan
Copy link
Owner

wokhan commented Apr 2, 2023

Hi @UltimateEvil, thank you for this report, I'll have a look asap (indeed it's easy to reproduce!).

@wokhan
Copy link
Owner

wokhan commented Apr 2, 2023

At least one reason was that a property of the EventReader was still bound when disposing the object (the "NewMatchingEntriesCount" one). I get way less issues now (will commit that in a few minutes).
I also changed the gridLog source to a regular XAML binding.

Btw, I got a lot of ExecutionEngineException today (it even froze my VS instance until killed, blocking the debugger I guess), I didn't before so something clearly changed (on .NET side or mine) and WFN doesn't like that...
I changed the way I asynchronously set values (using direct memory access through my Wokhan.Core library) so I guess it might all come from there.

I'm not closing this issue for now, I'll wait for your feedback first.

Note: I wrongly included issue #154 in the commit comment, but commit 5867959 is the one concerned by this comment. Binaries should be ready now.

@wokhan
Copy link
Owner

wokhan commented Apr 2, 2023

Confirmed: I updated the GetOrSetValueAsync extension method to use Unsafe methods (to manipulate backing field directly without reflection) and it proved unstable. I'm rolling back. Performance might get a hit though... (will work on that later, stability is way more important here).

@UltimateEvil
Copy link
Author

Thanks for the fast response.

Tested while building the latest version of the master branch as of now.

Some ExecutionEngineException have been popping up for me as well. Though I was not able to pin them down to anything specific, so I did not end up amking an issue out of it. Instead reporting the parts which could actually be fixed without voodoo magic.

This did fix a part of the issue. Though now in random intervals - also can be kind of reproduced by refreshing a lot -

Somehow EventLogAsyncReader's property eventLog is null when it is accessed. Any chance Dispose() suld be called before all async events are finished handling?

When I change the dispose() method to not set eventLog to null, I instead end up with the
System.ObjectDisposedException: Cannot access a disposed object. ObjectDisposed_ObjectName_Name exception.

Just feels like we are attempting to destroy the object before references to it are droped in all threads.

@wokhan
Copy link
Owner

wokhan commented Apr 2, 2023

I guess you're absolutely right 🙄 Looking into it.

Update: still not fixed, as I've been working on other issues. But preventing the refresh to occur when it's already processing somehow should help (as would stopping the "anticipated" object disposal / null setter you mentioned).

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