Hi Jon, On 12.08.2021 14:38, Sahananda Sahananda wrote: > Thanks for taking care of that. Sorry I started this.
Nothing to be sorry about! At the time it made sense for everyone else as well. (Only the methods in callbacks coming from different threads need to be unguarded such that they can run concurrently to another method on a different thread that may have caused that callback, and the like ..) Sometimes that is the fate of innovation: trying to improve something, but for unforseen side-effects stepping back and backing out. Most of the time the innovations stick! :) Cheers ---rony > > On Thu, 12 Aug 2021, 13:26 Rony G. Flatscher, <rony.flatsc...@wu.ac.at > <mailto:rony.flatsc...@wu.ac.at>> wrote: > > Backed out and as a reslut rejecting RFE 720, cf. > <https://sourceforge.net/p/oorexx/feature-requests/720/> > <https://sourceforge.net/p/oorexx/feature-requests/720/>. > > ---rony > > On 11.08.2021 13:21, Rony G. Flatscher wrote: >> On 10.08.2021 18:07, Rick McGuire wrote: >>> No, this is way too complicated to add. It would be better to just back >>> out the feature, >>> which was only added under the assumption that there was no detectable >>> effect in doing so. >>> The guarded/nonguarded state clearly protects more than just the >>> variable state of the >>> object, it also synchronizes the order of invocations of any methods >>> called from within the >>> method. >> >> +1 >> >> So would revert the changes applied as recorded in >> <https://sourceforge.net/p/oorexx/feature-requests/720/> >> <https://sourceforge.net/p/oorexx/feature-requests/720/> in reverse >> order from the svn root, >> like: >> >> svn merge -r 12141:12140 . >> svn ci -m "Backed out r12141." >> >> svn merge -r 12006:12005 . >> svn ci -m "Backed out r12006." >> >> Would that be the correct sequence? >> >> Anything to watch out, any suggestions otherwise? >> >> ---rony >> >>> >>> On Tue, Aug 10, 2021 at 11:30 AM Rony G. Flatscher >>> <rony.flatsc...@wu.ac.at >>> <mailto:rony.flatsc...@wu.ac.at>> wrote: >>> >>> To keep the best of both worlds I propose to allow to activate this >>> particular feature >>> (defaulting methods to unguarded if the first statement neither >>> starts with EXPOSE nor >>> with USE LOCAL) with a new option "unguarded" on the options >>> directive, i.e.: >>> >>> ::options unguarded >>> >>> Enclosed please find the intended patch (tested) with request for >>> comment. >>> >>> Will update the documentation (rexxref) and the method directive >>> test unit accordingly. >>> >>> ---rony >>> >>> P.S.: With the patch applied the following program: >>> >>> say 'OPTIONS UNGUARDED set ...' >>> >>> do i=1 to 5 >>> m=.Test1~method("M"i) >>> say "m"i"~isGuarded:" m~isGuarded >>> end >>> >>> ::options unguarded >>> >>> ::class Test1 >>> >>> ::method m1 >>> say 'm1' >>> >>> ::method m2 >>> expose a >>> >>> ::method m3 >>> use local >>> >>> ::method m4 guarded >>> >>> ::method m5 unguarded >>> >>> will yield the following (expected) output: >>> >>> OPTIONS UNGUARDED set ... >>> m1~isGuarded: 0 >>> m2~isGuarded: 1 >>> m3~isGuarded: 1 >>> m4~isGuarded: 1 >>> m5~isGuarded: 0 >>> >>> Without the new "unguarded" subdirective of the options directive >>> in effect method m1 >>> will be guarded by default (the original behavior). >>> >>> >>> On 07.08.2021 18:28, Rony G. Flatscher wrote: >>>> >>>> Sleeping and thinking more over it I would suggest to remove this >>>> feature altogether! >>>> The major reasing being that the Rexx philosophy (and ooRexx by >>>> extension) should make >>>> coding as easy as possible for programmers. >>>> >>>> The default for methods (and related) has always been "guarded" >>>> such that one would not >>>> have to write "guarded" to the method directive. With that default >>>> only in very rare >>>> cases (in multithreaded scenarios) would one have to write >>>> "unguarded" to a method >>>> directive. And if doing so one must have an understanding of >>>> multithreading and the >>>> ramifications if someone would want a method to run unguarded. >>>> >>>> Compare this to the current situation with this new feature on: in >>>> order to fix >>>> BSF.CLS all of a sudden I had to add the keyword "guarded" to >>>> every method to gain the >>>> default behaviour for ooRexx < 5.0 and thereby re-enabling correct >>>> execution of GUI >>>> nutshell examples. >>>> >>>> Ad feature: originally it was thought to be helpful to programmers >>>> by saving the >>>> programmers to write "unguarded" to a method directive for a >>>> method that is known to be >>>> safe in unguarded mode thinking that methods that have no direct >>>> access to the >>>> attribute pool (i.e. the method routine would not start with >>>> "expose" or "use local") >>>> qualify for unguarded execution not thinking about scenarios where >>>> this is not enough. >>>> >>>> To make it easy for the programmer (i.e. not having to know >>>> additional, sometimes quite >>>> subtle, concepts that play a role in this context) I would be in a >>>> strong favor to >>>> leave the default "guarded" in place. Then either remove this new >>>> feature altogether or >>>> make it an explicit option a programmer has to state ("::OPTIONS" >>>> with a new pair >>>> "guarded|unguarded"). >>>> >>>> What opinions have others? Do you concur? >>>> >>>> ---rony >>>> >>>> >>>> On 06.08.2021 15:09, Rony G. Flatscher wrote: >>>>> >>>>> Background: In order for ooRexx programmers getting acquainted >>>>> with BSF4ooRexx/Java >>>>> quickly there are numerous nutshell examples in >>>>> "bsf4rexx/samples". Most of these >>>>> nutshell examples stem from observations of students over time >>>>> who should get help by >>>>> demonstrating them how to achieve something of interest with >>>>> these (mostly brief) >>>>> nutshell examples. >>>>> >>>>> One interesting problem has been the interaction from ooRexx with >>>>> GUI objects which >>>>> must be carried out on the GUI threads in Java (the "awt thread" >>>>> or the "JavaFX >>>>> Application thread"). Although they got the necessary information >>>>> about the >>>>> architecture and what to do in ordert to become able to send >>>>> messages on GUI threads, >>>>> they kept running into problems, losing a lot of time (even >>>>> months because they could >>>>> not get it working in more complex programs). >>>>> >>>>> To make a long story short, I came up with a message based >>>>> solution, that was very >>>>> easy to understand and to employ for them. None of the students >>>>> ran into the GUI >>>>> thread problems since then. >>>>> >>>>> The solution is an ooRexx class for awt (the Java "abstract >>>>> windows toolkit") named >>>>> .AwtGuiThread and for JavaFX (a powerful GUI system) >>>>> .FxGuiThread, both subclassing a >>>>> common superclass .AbstractGuiThread. These classes allow one to >>>>> send the ooRexx >>>>> message runLater[Latest](GUIreceiver, messageName, arguments) >>>>> which get queued and >>>>> dispatched on the GUI thread later. >>>>> >>>>> The nutshell examples >>>>> >>>>> "bsf4rexx/samples/3-090_update_awtSwing_GUI-from-non-GUI-thread.rxj" and >>>>> >>>>> "bsf4rexx/samples/JavaFX/javafx_update_GUI-from-non-GUI-thread.rxj" >>>>> demonstrate how to >>>>> employ this infrastructure. They have been working for years >>>>> without a problem. >>>>> >>>>> While working on BSF4ooRexx I stumbled over an error (not having >>>>> run those two >>>>> examples for quite some time) which seems to indicate that ooRexx >>>>> now creates an error >>>>> when being used from different threads: >>>>> >>>>> >>>>> F:\work\svn\bsf4oorexx\trunk\bsf4oorexx\samples>3-090_update_awtSwing_GUI-from-non-GUI-thread.rxj >>>>> screenSize: [java.awt.Dimension[width=1920,height=1080]] >>>>> winSize : [java.awt.Dimension[width=800,height=200]] >>>>> xPos=[560] yPos=[440] >>>>> a REXXEVENTHANDLER::actionPerformed - starting Rexx thread >>>>> The SOME_REXX_CLASS class::updateGuiFromRexxThread - just >>>>> arrived, GUI thread: 23808 >>>>> The SOME_REXX_CLASS class::updateGuiFromRexxThread - now >>>>> running on thread: 7428 >>>>> *-* Compiled method "DELETE" with scope "Queue". >>>>> 5727 *-* msgQueue~delete(idx) -- delete the guiMsg >>>>> object >>>>> 5637 *-* forward message "REMOVEMESSAGE" continue -- >>>>> remove all GUI messages of the same name targeted to the same object >>>>> 207 *-* .AwtGuiThread~runLaterLatest(label, "setText", >>>>> "i", str) >>>>> Error 93 running >>>>> F:\work\svn\bsf4oorexx\trunk\bsf4oorexx\samples\3-090_update_awtSwing_GUI-from-non-GUI-thread.rxj >>>>> line 207: Incorrect >>>>> call to method. >>>>> Error 93.966: Incorrect queue index "1". >>>>> a REXXEVENTHANDLER::windowClosing - release lock >>>>> ('closeApp=.true') which will allow a blocked 'waitForExit' method to >>>>> resume and return >>>>> >>>>> F:\work\svn\bsf4oorexx\trunk\bsf4oorexx\samples> >>>>> >>>>> and >>>>> >>>>> >>>>> F:\work\svn\bsf4oorexx\trunk\bsf4oorexx\samples\JavaFX>javafx_update_GUI-from-non-GUI-thread.rxj >>>>> a REXXBUTTONHANDLER::handle - starting Rexx thread >>>>> The SOME_REXX_CLASS class::updateGuiFromRexxThread - just >>>>> arrived, GUI thread: 24244 >>>>> The SOME_REXX_CLASS class::updateGuiFromRexxThread - now >>>>> running on thread: 14124 >>>>> *-* Compiled method "DELETE" with scope "Queue". >>>>> 5727 *-* msgQueue~delete(idx) -- delete the guiMsg >>>>> object >>>>> 5637 *-* forward message "REMOVEMESSAGE" continue -- >>>>> remove all GUI messages of the same name targeted to the same object >>>>> 194 *-* .FxGuiThread~runLaterLatest(label, "setText", >>>>> "i", str) >>>>> Error 93 running >>>>> F:\work\svn\bsf4oorexx\trunk\bsf4oorexx\samples\JavaFX\javafx_update_GUI-from-non-GUI-thread.rxj >>>>> line 194: Incorrect call to method. >>>>> Error 93.966: Incorrect queue index "1". >>>>> >>>>> The ooRexx code where the error occurs looks like, lines # 5637 >>>>> and # 5727 are >>>>> highlighted in green and bold: >>>>> >>>>> /* >>>>> ------------------------------------------------------------------------------------------------- >>>>> */ >>>>> -- method replaces existing target: this way only the >>>>> latest sent message will get executed! >>>>> /** This class method allows to define a Rexx message with >>>>> its arguments that should >>>>> * be processed on the GUI thread. Each invocation will >>>>> create a new <code>GUIMessage</code> >>>>> * from the supplied arguments and returns it for further >>>>> inspection in addition to queueing >>>>> * it for later execution on the GUI thread. Unlike >>>>> <code>runLater</code> this method will >>>>> * first remove any queued messages with the same target and >>>>> the same message name, before >>>>> * queueing this message. >>>>> * >>>>> * @param target the target object to receive a message on >>>>> the GUI thread >>>>> * @param messageName the message name to send to the target >>>>> on the GUI thread >>>>> * @param indicator optional; indicates with "I" >>>>> (Indivdual) that the arguments >>>>> * are listed individually, "A" >>>>> (Array) indicates that the fourth >>>>> * argument is an array containing the >>>>> arguments to supply with the >>>>> * message on the GUI thread >>>>> * >>>>> * @return GUIMessage a GUI message object (modelled after >>>>> ooRexx' <code>Message</code> class >>>>> * that allows one to inspect the state of >>>>> the message (process completed, >>>>> * fetching a possible result, determining >>>>> whether an error occurred and >>>>> * inspecting it) >>>>> */ >>>>> ::method runLaterLatest class >>>>> use strict arg target, messageName, ... >>>>> >>>>> signal on syntax >>>>> if self~JavaGuiUtilityClz=.nil then self~setup -- make >>>>> sure attributes are initialized >>>>> >>>>> *forward message "REMOVEMESSAGE" continue -- remove all GUI >>>>> messages of the same >>>>> name targeted to the same object* >>>>> guiMsg=result -- fetch returned GUIMessage >>>>> object >>>>> msgQueue=self~msgQueue -- get message queue >>>>> msgQueue~queue(guiMsg) -- now enqueue current (latest) >>>>> guiMsg >>>>> >>>>> if self~waitingToRun=.false then -- "runLater" not invoked >>>>> yet? >>>>> do >>>>> self~waitingToRun=.true >>>>> self~invokeOnGuiThread -- make sure Java will dispatch >>>>> Rexx message on GUI thread >>>>> end >>>>> return guiMsg -- allows caller to get a hold of >>>>> a possible return value >>>>> >>>>> syntax: >>>>> raise propagate >>>>> >>>>> ... cut ... >>>>> >>>>> /* >>>>> ------------------------------------------------------------------------------------------------- >>>>> */ >>>>> -- method removes all GUI message objects >>>>> /** This private class method removes all GUIMessage objects >>>>> from the message queue that will be processed >>>>> * later when Java call backs on the GUI thread, that have >>>>> the supplied target and the supplied >>>>> * message name. >>>>> * >>>>> * @param target the target object to receive a message on >>>>> the GUI thread >>>>> * @param messageName the message name to send to the target >>>>> on the GUI thread >>>>> * @param indicator optional; indicates with "I" >>>>> (Indivdual) that the arguments >>>>> * are listed individually, "A" >>>>> (Array) indicates that the fourth >>>>> * argument is an array containing the >>>>> arguments to supply with the >>>>> * message on the GUI thread >>>>> * >>>>> * @return GUIMessage a GUI message object (modelled after >>>>> ooRexx' <code>Message</code> class >>>>> * that allows one to inspect the state of >>>>> the message (process completed, >>>>> * fetching a possible result, determining >>>>> whether an error occurred and >>>>> * inspecting it) >>>>> */ >>>>> ::method removeMessage class private >>>>> use strict arg target, messageName, ... >>>>> >>>>> signal on syntax >>>>> if self~JavaGuiUtilityClz=.nil then self~setup -- make >>>>> sure attributes are initialized >>>>> >>>>> -- remove all occurrences of messages with the same >>>>> messagename and the same target >>>>> msgQueue=self~msgQueue -- get message queue >>>>> idx=msgQueue~first -- get index of the first item >>>>> in the queue, if any >>>>> do while idx<>.nil -- a valid index in hand ? >>>>> tmpItem=msgQueue~at(idx) -- fetch item, a GuiMessage >>>>> -- same target, same messagename? >>>>> if tmpItem~target=target, >>>>> tmpItem~messageName~caselessEquals(messagename) then >>>>> *msgQueue~delete(idx) -- delete the guiMsg object*** >>>>> idx=msgQueue~next(idx) -- get index of next item in >>>>> the queue >>>>> end >>>>> >>>>> guiMsg=self~createGuiMessage(arg(1,"array")) -- create and >>>>> return GUI message object >>>>> return guiMsg -- allows caller to get a hold of >>>>> a possible return value >>>>> >>>>> syntax: >>>>> raise propagate >>>>> >>>>> None of the methods of these three classes are marked as >>>>> unguarded such that in the >>>>> past if one method executes no other can do so in parallel, >>>>> inhibiting concurrent >>>>> access of the queue. >>>>> >>>>> However, I recall that there was a change in ooRexx 5.0 that if >>>>> methods do not have an >>>>> expose keyword statement than unlike earlier versions of ooRexx >>>>> such expose-less >>>>> methods will be made unguarded. >>>>> >>>>> Yet, as in this real use case this change may bring >>>>> incompatibilities for existing >>>>> programs (BSF4ooRexx has been around for more than 10 years). The >>>>> problem lies in >>>>> getting and using the queue in different threads because of this >>>>> (at the time of >>>>> devising the solution unforeseen) as no default blocking of >>>>> methods takes place. >>>>> >>>>> To double-check I added manually "guarded" to all methods of the >>>>> classes that play >>>>> together here and doing so, made the nutshell programs work again. >>>>> >>>>> --- >>>>> >>>>> The reason I bring this forward is twofold: >>>>> >>>>> * the change may break existing programs and programmers may be >>>>> clueless as to why >>>>> that happens and therefore not know how to solve it, >>>>> * it should be discussed whether keeping this change, but if so >>>>> it needs to be >>>>> prominently communicated as the cause may be too subtle for >>>>> many, especially if >>>>> using ooRexx code for which they do not have the source: >>>>> o one solution may be to have a new OPTION unguared|guarded >>>>> determining how >>>>> expose-less methods should be treated; this way this >>>>> default to unguarded >>>>> behavior needs to be activated explicitly making the >>>>> programmer aware of it, >>>>> yet older programs still would function with the default >>>>> to guarded behavior >>>>> that was in place in erarlier versions of ooRexx, >>>>> o another solution would be to not only analyze whether the >>>>> expose statement >>>>> exists, but in the case where it does not exist also >>>>> analyze whether messages >>>>> to self occur and if so leave the method guarded; however >>>>> this bears the >>>>> problem that one could use an object that refers to self >>>>> such that the problem >>>>> might surface nevertheless. >>>>> >>>>> Any thoughts, ideas, suggestions, conclusions? >>>>> >>>>> ---ron >>>>>
_______________________________________________ Oorexx-devel mailing list Oorexx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/oorexx-devel
