Discussion :

[agparchitecture]

Bonsoir à tous,

Je rencontre un problème durant l'éxécution avec le code suivant exécuté dans un backgroundworker :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
		void bckworker_DoWork(object sender, DoWorkEventArgs e)
		{
			Parallel.ForEach<DataGridViewRow>(
				tmp,
				po,
				x => CheckRow(x)
			);
		}
 
async void CheckRow(DataGridViewRow row)
		{
			Domaine dom = dtgvListDomaines.Rows[row.Index].Cells[CELLHREF].Tag as Domaine;
 
			bool hostExist = MarkNet.Helpers.CheckHlp.DNSExist(dom);
 
			if(hostExist == true)
			{
 
				HttpClient client = new HttpClient();
				HttpResponseMessage response = new HttpResponseMessage();
				try
				{
					response = await client.GetAsync("http://" + dom.Name);
					string responseUri = response.RequestMessage.RequestUri.ToString();
 
					if(responseUri != ("http://" + dom.Name + "/"))
					{
						dom.CheckInfo.Redirection = responseUri;
					}
				}
				catch (Exception ex) //TODO: voir erreur timeout, 403, ....
				{
					dom.CheckInfo.Redirection = response.StatusCode.ToString();
				}	
			}
				bckworker.ReportProgress(0, new object[] {row.Index, hostExist, dom.CheckInfo.Redirection});
		}

l'erreur est la suivante :

System.InvalidOperationException: OperationCompleted a déjà été appelé pour cette opération. D'autres tentatives d'appel ne seraient pas conformes.

à System.ComponentModel.AsyncOperation.VerifyNotCompleted()
à System.ComponentModel.AsyncOperation.Post(SendOrPostCallback d, Object arg)
à System.ComponentModel.BackgroundWorker.ReportProgress(Int32 percentProgress, Object userState)
à WebMark.Gui.Controls.PLGCheckDomaines.<CheckRow>d__1.MoveNext() dans c:\Users\AGPAdmin\Desktop\Perso C#\Webmark\Plugins\DomaineCheck\DomaineCheck\CheckDomaines.cs:ligne 245
--- Fin de la trace de la pile à partir de l'emplacement précédent au niveau duquel l'exception a été levée ---
à System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
à System.Runtime.CompilerServices.AsyncMethodBuilderCore.<>c.<ThrowAsync>b__6_1(Object state)
à System.Threading.QueueUserWorkItemCallback.WaitCallback_Context(Object state)
à System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
à System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
à System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
à System.Threading.ThreadPoolWorkQueue.Dispatch()
à System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

et la ligne 245 est :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

bckworker.ReportProgress(0, new object[] {row.Index, hostExist, dom.CheckInfo.Redirection});

Ce que je ne comprend pas c'est pourquoi l'appel fonctionne bien et parfois (pas de moment particulier vu) le code plante .

d'avance merci pour votre aide pour permettre le bon fonctionnement du code.

[François DORIN]

Bonjour,

Je pense que le problème vient de l'usage de async/await. Peux-tu réessayer sans et dire si cela fonctionne correctement ?

[agparchitecture]

J'ai essayé :

sans mettre le async à checkRow mais le compilateur indique une erreur : L'opérateur 'await' peut seulement être utilisé dans une méthode async. Marquez cette méthode avec le modificateur 'async' et changez son type de retour en 'Task'. (CS4033) -

sans le await : Impossible de convertir implicitement le type 'System.Threading.Tasks.Task<System.Net.Http.HttpResponseMessage>' en 'System.Net.Http.HttpResponseMessage' (CS0029) : je ne sais pas comment convertir

et je n'ai pas trouvé l'équivalent à httpclient.getasync sans le async?

Un petit conseil?

[François DORIN]

Tu peux simplement modifier le code comme suit :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
// La ligne suivante est à remplacer
//response = await client.GetAsync("http://" + dom.Name);
Task<HttpResponseMessage> task = client.GetAsync(("http://" + dom.Name);
response = task.Result;

Tu enlèves ainsi le await. Ne pas oublier non plus de supprimer le async au niveau de la déclaration (bon, ça devrait marcher sans, mais tu auras un avertissement du compilateur à cause d'une méthode marqué avec async s'il n'y a pas de await.

Maintenant, que j'ai un peu plus de temps, voici ce que je pense être le problème. Tu as un background worker qui parallélise des calculs via un Parallel.ForEach(. Jusque là, rien d'anormal.
Maintenant, dans la boucle parallélisée, tu fais appel à la méthode CheckRow. Cette méthode est marquée avec le mot clé async. Cela signifie qu'à un moment donné, la fonction va se mettre en attente d'une tâche, via le mot clé await. Et c'est là que réside le problème ! Car avec async/await, lorsque ta fonction se met en attente, l'exécution est rendu au code appelant ! Donc, à la méthode qui est appelée par la boucle parallélisée. Comme cette méthode ne fait rien d'autre, cette itération est marquée comme terminée, alors même que l'exécution de CheckRow n'est pas encore finie.

Cela signifie que la boucle parallélisée peut se terminer alors que les calculs ne le sont pas. Une fois la boucle terminée, cela met fin au background worker car la méthode bckworker_DoWork aura terminé son exécution. 3secondes plus tard, tu reçois une réponse à une requête, la méthode CheckRow correspondante peut donc continuer son exécution. Lorsqu'elle essait de mettre à jour le background worker, cela génère une exception car le background worker est déjà terminé.

La modification que je te conseille va rendre le code de la méthode CheckRow bloquant (sans rendre la main à l'appelant), évitant donc le problème mentionné ci-dessus.

[Noxen]

Ça n'est pas un peu redondant ou contradictoire d'utiliser simultanément BackgroundWorker + Parallel.ForEach + await / async (dans le genre thé et café) ?

Par contre la déclaration d'une méthode async void Methode() n'est pas canonique. Une méthode async devrait avoir en type de retour en Task ou Task<T> afin de pouvoir être manipulées par l'API des Task. async void a été implémentée afin de répondre à un besoin spécifique : les événements asynchrones.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
async void ProcessEvent(object sender, EventArgs args)
{
    // traitement très long
}

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
var bw = new BackgroundWorker();
bw.ProgressChanged += ProcessEvent;

Dans le cas d'un événement on pourrait contourner le problème par exemple en reportant le traitement dans un Task.Run(), mais le fait de bénéficier d'async dans ce cas là permet d'assurer la propagation d'async depuis des méthodes appelées par l'événement, avec en plus la possibilité de travailler directement sur le thread de l'événement.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
static async Task<string> ProcessFile(string filePath)
{
  string result = null;
  // ...
  var tasks = new List<Task<int>>();
  foreach (var line in lines) tasks.Add(Task.Run(() => ProcessLine(line)));
  var task = Task.WhenAll(tasks);
  var data = await task;
  // ...
  return result;
}

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
this.BtnStart.Click += async (_, __) => // une expression lambda peut être déclarée async
{
  this.BtnStart.Enabled = false;
  var text = await ProcessData(this.TxtFilePath.Text);
  this.TxtResult = text;
  this.BtnStart.Enabled = true;
}

[François DORIN]

Ça n'est pas un peu redondant ou contradictoire d'utiliser simultanément BackgroundWorker + Parallel.ForEach + await / async (dans le genre thé et café) ?

Pas du tout. D'un côté, on a une tâche longue. De l'autre, on souhaite accélérer cette tâche via des traitements simultanées. Rien de contradictoire.

Par contre la déclaration d'une méthode async void Methode() n'est pas canonique. Une méthode async devrait avoir en type de retour en Task ou Task<T> afin de pouvoir être manipulées par l'API des Task. async void a été implémentée afin de répondre à un besoin spécifique : les événements asynchrones.

Elle n'a pas été implémentée pour répondre spécifiquement à ce besoin. C'est un usage courant, certes, mais pas unique. Dès lors que la tâche ne renvoie pas de résultat et/ou que d'autres méthodes de synchronisation sont utilisées (ici, notification via le background worker), alors l'usage est tout à fait possible. Je rajouterais de plus que s'il avait fait une méthode retournant un Task, alors le débuggage aurait été beaucoup plus pénible !

En effet, avec un type de retour qui est void, toute exception générée met fin immédiatement au programme. Avec un Task ou Task<T>, l'exception est bien générée mais n'est propagée qu'à l'accès de l'instance de Task retournée. Comme il n'y aurait jamais accédé, il aurait vu que son code ne faisait pas ce qu'il attendait, sans vraiment savoir pourquoi.

Dans le cas d'un événement on pourrait contourner le problème par exemple en reportant le traitement dans un Task.Run(), mais le fait de bénéficier d'async dans ce cas là permet d'assurer la propagation d'async depuis des méthodes appelées par l'événement, avec en plus la possibilité de travailler directement sur le thread de l'événement.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
static async Task<string> ProcessFile(string filePath)
{
  string result = null;
  // ...
  var tasks = new List<Task<int>>();
  foreach (var line in lines) tasks.Add(Task.Run(() => ProcessLine(line)));
  var task = Task.WhenAll(tasks);
  var data = await task;
  // ...
  return result;
}

Attention, il a des précautions à prendre. Ce code, en fonction du contexte, peut mener à un blocage de l'application ! par exemple, si cette méthode est appelé depuis un thread graphique. Il ne faut jamais mélanger l'usage de async/await et de méthodes classiques comme Task.Wait, Task.WhenAll, etc... sans précaution. Cela peut mener à des situations très difficile à déboguer... Voir l'article que j'ai écris à ce sujet.

Bon, euh l'autre c'est ce qu'il a fait dans son exemple mais il dit qu'il faut pas le faire ?!!!

C'est vrai, mais dans mon exemple, je me suis assuré que c'était approprié. Dans un background worker, on peut le faire sans soucis, car on utilise le pool de threads.

[agparchitecture]

Bon pour un peu d'éclaircissements du pourquoi j'ai faits ce code.

En fait j'ai une interface graphique avec un datagridview qui contient une liste de domaine à vérifier. Je souhaitais le faire en mettant à jour l'interface graphique et de ne pas me limiter à une vérification une à une.

Je me suis donc lancé dans le parallel.foreach pour le traitement en parrallèle. Cependant, je me retrouvais avec de multiple déléguate pour la gestion de l'affichage (progressbar, datagridview, bouton, ...). Ce nombre de délégate me paraissait for lourd.

J'ai donc utilisé le backgroundworker pour ne pas fichier l'affichage et le gros avantage c'st que le progresswork est renvoyé sur le thread graphique m'évitant les délégates.

Etant autodidacte, est-ce que cette méthode et réflexion est convenable? Ce n'est pas évident de voir si le code est bon sans retour. En tout cas le code fait ce que j'attendais depuis les modifications....

Merci pour l'aide.

[Noxen]

Elle n'a pas été implémentée pour répondre spécifiquement à ce besoin. C'est un usage courant, certes, mais pas unique.

Pour le coup ce n'est pas moi qui le dit mais Stephen Cleary dans un article (de 2013) :

Void-returning async methods have a specific purpose: to make asynchronous event handlers possible.

C'est vrai qu'on est pas techniquement obligé de se limiter aux événements, mais personnellement je préfère rester sur une pratique standardisée.

EDIT: ça demanderait réflexion dans un contexte "fire and forget", où l'on veut juste lancer la méthode sans se préoccuper du résultat. Je verrai bien quand le cas se présentera à moi.

Attention, il a des précautions à prendre. Ce code, en fonction du contexte, peut mener à un blocage de l'application ! par exemple, si cette méthode est appelé depuis un thread graphique. Il ne faut jamais mélanger l'usage de async/await et de méthodes classiques comme Task.Wait, Task.WhenAll, etc... sans précaution. Cela peut mener à des situations très difficile à déboguer... Voir l'article que j'ai écris à ce sujet.

Effectivement, si une méthode async est appelée par depuis thread GUI la combinaison await dans la méthode async + Task.Wait() dans l’appelant va provoquer un verrou sur le thread GUI, car l'appelant attend que l'async se termine et l'async attend que l'appelant libère le thread pour se terminer. Le moyen d'échapper à cela est d'utiliser ConfigureAwait(continueOnCapturedContext: false) afin que la méthode async poursuive non sur le contexte d'exécution capturé (le thread GUI) mais sur le pool de thread. C'est une notion que je n'avais pas encore intégrée lors de mes lectures précédentes.

[Noxen]

Etant autodidacte, est-ce que cette méthode et réflexion est convenable?

Ça me parait tout à fait correct comme approche. Tu es sur du Winform ?

[agparchitecture]

oui je suis bien en winform. Je sais qu'il y a xaml mais comme j'ai des machines avec windows en dual boot avec linux, je prefere rester sur du winform car ca me permettra éventuellement de porte mes programmes sur linux avec mono sans pour autant recoder toute l'interface graphique.

[François DORIN]

Etant autodidacte, est-ce que cette méthode et réflexion est convenable?

Tout à fait. C'est même pour ça que les background worker ont été fait !

Ce n'est pas évident de voir si le code est bon sans retour. En tout cas le code fait ce que j'attendais depuis les modifications....

Autrement dit, problème résolu ?

C'est vrai qu'on est pas techniquement obligé de se limiter aux événements, mais personnellement je préfère rester sur une pratique standardisée.

EDIT: ça demanderait réflexion dans un contexte "fire and forget", où l'on veut juste lancer la méthode sans se préoccuper du résultat. Je verrai bien quand le cas se présentera à moi.

Un gestionnaire d'évènement asynchrone reste l'application principale des méthodes async void. Mais il y a quelques cas où je préfère l'utiliser, comme celui où on est. Comme je le disais, en cas d'erreur, on le sait tout de suite ! Si on avait utilisé la variant async Task, voir qu'une exception était générée aurait demandé beaucoup plus d'investigation.

Effectivement, si une méthode async est appelée par depuis thread GUI la combinaison await dans la méthode async + Task.Wait() dans l’appelant va provoquer un verrou sur le thread GUI, car l'appelant attend que l'async se termine et l'async attend que l'appelant libère le thread pour se terminer. Le moyen d'échapper à cela est d'utiliser ConfigureAwait(continueOnCapturedContext: false) afin que la méthode async poursuive non sur le contexte d'exécution capturé (le thread GUI) mais sur le pool de thread. C'est une notion que je n'avais pas encore intégrée lors de mes lectures précédentes.

C'est effectivement une autre possibilité.

oui je suis bien en winform. Je sais qu'il y a xaml mais comme j'ai des machines avec windows en dual boot avec linux, je prefere rester sur du winform car ca me permettra éventuellement de porte mes programmes sur linux avec mono sans pour autant recoder toute l'interface graphique.

Dans ce cas, reste effectivement sur du winform. C'est relativement bien portable. J'ai déjà testé une belle application client lourd sous linux et mono "pour le fun" il y a quelques années. Ca passait pas trop mal Des petites différences subtiles parfois (sur les séquences de déclenchements des évènements notamment, sur un DataGridView de mémoire), mais rien ne nécessitant une réécriture complète de l'application.