RFR: JMC-6555 Convert JOverflow plugin to SWT
Mario Torre
neugens at redhat.com
Mon Aug 26 13:30:02 UTC 2019
On Mon, Aug 26, 2019 at 3:24 PM Mario Torre <neugens at redhat.com> wrote:
>
>
>
> On Mon, Aug 26, 2019 at 3:16 PM Jie Kang <jkang at redhat.com> wrote:
>>
>>
>> The thread safety of this construct is concerning. Is it expected to
>> only be called by a single thread? If yes, I'd appreciate a comment
>> specifying that it is not thread-safe, but it is okay because no more
>> than one thread calls this. If no, then I think a lock would be more
>> appropriate than a boolean.
>>
>> Also there is a guard in updateModel() but no guard in reset(); is
>> that intentional? Can things go awry if reset occurs while updateModel
>> is executing? I guess these kinds of questions need only be considered
>> if there is multi-threaded access...
>
>
> I'm not an expert of the Eclipse threading model, but I would be very surprised if the UI was updated outside the rendering thread, and afaik SWT does not allow access from multiple thread on its component hierarchy, so I would rather add a comment if this code was meant to be accessed by multiple threads since that would be the exception.
P.S.
In the case where the model is updated externally of the event thread,
you will likely need in the calling code some syncing libraries like
Display.asyncExec (or whatever Eclipse version of invokeLater is).
Indeed, the mIsUpdatingModel throws me off too, since it seems to
imply the view is being update concurrently.
Cheers,
Mario
--
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30 9205 5D7E 4952 3F65 7898
More information about the jmc-dev
mailing list