<div dir="auto">Mario,<div dir="auto">Do you approve the latest webrev, as it is ?</div><div dir="auto">Let's move forward, after this patch integrated.</div><div dir="auto">Laurent</div></div><br><div class="gmail_quote"><div dir="ltr">Le lun. 1 oct. 2018 Ã  11:17, Jiri Vanek <<a href="mailto:jvanek@redhat.com">jvanek@redhat.com</a>> a Ã©crit :<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">...<br>
> <br>
>  Â  Â  >  Â  Â 3. It seems that methods that use "observable" are synchronized.<br>
>  Â  Â  >  Â  Â However, the EDT will use "observable" and it's not synchronized. Is<br>
>  Â  Â  >  Â  Â this safe?<br>
>  Â  Â  ><br>
>  Â  Â  ><br>
>  Â  Â  > Ok, fixed:<br>
>  Â  Â  > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  synchronized(observable) {<br>
>  Â  Â  > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  if (observable.hasChanged() ||<br>
>  Â  Â  > (Boolean.TRUE.equals(force))) {<br>
>  Â  Â  > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  observable.notifyObservers(force);<br>
>  Â  Â  > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  }<br>
>  Â  Â  > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  }<br>
> <br>
>  Â  Â Right, but I don't think this addresses the issue, one is synchronized<br>
>  Â  Â on the observable, the other methods are on the JavaConsole, so we<br>
>  Â  Â should probably pick one.<br>
> <br>
> <br>
> Sorry, I did not catch the problem.<br>
> My fix is very minimal and this patch does not fix synchronization issues already present in ITW.<br>
> I tested javaws with the console opened and did not notice any problem.<br>
> <br>
> <br>
>  Â  Â By going through the code though, it seems like we could just ensure<br>
>  Â  Â that updateModel() always run in the EDT and remove all other<br>
>  Â  Â synchronised keywords from the methods, the only user of this method<br>
>  Â  Â which can execute outside the EDT is addMessage, but otherwise all other<br>
>  Â  Â consumers are executing in the EDT.<br>
> <br>
> <br>
> Could we postpone this point later as I did not want to rewrite too much the JavaConsole ?<br>
> You propose to remove all 'synchronized' keywords in JavaConsole, I am pretty sure the <br>
<br>
I would strongly advice against removing the synchronizations. The logging bottleneck is lazily <br>
initiated singleton, and there were many troubles without it. Maybe with plugin gone, the troubles <br>
will decrease a lot, But i doubt.<br>
<br>
On contrary:<br>
<br>
 >on the observable, the other methods are on the JavaConsole, so we<br>
 >  Â  Â should probably pick one.<br>
<br>
Sounds like straightforward fix. Anyway, this is on your judgement.<br>
<br>
> synchronization overhead does not hurt here...<br>
> If you could propose an alternative, I am OK to integrate it.<br>
<br>
Please, as another changeset (if any).<br>
<br>
<br>
<br>
Thanx!<br>
  Â J.<br>
</blockquote></div>