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

Word Export: prevent cross-thread exception #184

Merged
merged 18 commits into from
Jan 16, 2025
Merged

Conversation

mark-sil
Copy link
Contributor

@mark-sil mark-sil commented Oct 16, 2024

When disposing the ClerkActivator do not update the status bar in the call to ActivateUI().
XHTML export is also calling this from a different thread but we were not seeing this error because the text string was the same as the current value so no change was made.
Notes:
In OnExport(), after the command completes, we call ActivateUI() a second time, and this time we do update the status bar.
The exception was only happening when there were reversals and it seemed to only/mostly happen in debug builds.

Change-Id: Ife8957181e5a07736b1f56e2f488aa92c8f226c5


This change is Reviewable

When disposing the ClerkActivator do not update the status bar
in the call to ActivateUI().
XHTML export is also calling this from a different thread but we
were not seeing this error because the text string was the same
as the current value so no change was made.
Notes:
In OnExport(), after the command completes, we call
ActivateUI() a second time, and this time we do update the
status bar.
The exception was only happening when there were reversals
and it seemed to only/mostly happen in debug builds.

Change-Id: Ife8957181e5a07736b1f56e2f488aa92c8f226c5
@jasonleenaylor
Copy link
Contributor

Src/xWorks/DictionaryExportService.cs line 194 at r1 (raw file):

				System.Diagnostics.Debug.WriteLineIf(!disposing, "****** Missing Dispose() call for " + GetType() + " ******");
				if (disposing && m_currentClerk != null && !m_currentClerk.IsDisposed)
					m_currentClerk.ActivateUI(true, false);

So if this is an exception that is happening because it is trying to modify winforms UI code from a different thread there is a more robust solution closer to where the control is modified. Every control has an InvokeRequired property that tells you if you are trying to execute something on the wrong thread. You can then use an Invoke method to get it to execute the code you want.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Lets look at another option first.

Reviewable status: 0 of 1 files reviewed, all discussions resolved

Copy link
Contributor Author

@mark-sil mark-sil left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @jasonleenaylor)


Src/xWorks/DictionaryExportService.cs line 194 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

So if this is an exception that is happening because it is trying to modify winforms UI code from a different thread there is a more robust solution closer to where the control is modified. Every control has an InvokeRequired property that tells you if you are trying to execute something on the wrong thread. You can then use an Invoke method to get it to execute the code you want.

I implemented the solution you suggested. Much better, thanks!

Copy link
Contributor Author

@mark-sil mark-sil left a comment

Choose a reason for hiding this comment

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

I implemented the solution you suggested.

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mark-sil)

@mark-sil mark-sil merged commit 9cb20de into release/9.1 Jan 16, 2025
5 checks passed
@mark-sil mark-sil deleted the cross-thread branch January 16, 2025 17:00
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.

4 participants