Hm...
Doesn't it become nearly impossible to protect against malicious intent, the
moment we make it extensible?
On Sunday, January 26, 2014, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> A malicious app could do
>
> for (int i=0; i < 100000; ++i) {
> new Level(“Level” + i, 1000 + i){};
> }
>
> Sure idiots can do lots of bad things but I don’t think Levels should be
> quite that flexible.
>
> Ralph
>
> On Jan 25, 2014, at 9:39 PM, Remko Popma <remko.po...@gmail.com> wrote:
>
> I don't think client code can do new Level(){} as the constructor requires
> String and int arguments.
>
> By the way, I am unclear on what went wrong with the enum approach you
> originally took.
> You said:
> StdLevel isn’t a Level because it can’t extend it if it is an enum,
> so I can’t initialize the levels using that.
>
> I don't understand. StdLevel implements the Level interface, right? So
> what do you mean by "it can't extend it"?
>
> The reason I ask is that for the code generation, "real" java enums may be
> easier to deal with than the extensible enum approach.
>
>
> On Sun, Jan 26, 2014 at 2:30 PM, Ralph Goers
> <ralph.go...@dslextreme.com>wrote:
>
> Out of curiosity, what exactly is the benefit of declaring the class
> abstract when it has a protected constructor? It seems like all you are
> accomplishing is making the instantiation syntax uglier. It also bothers me
> that open code can just do a new Level(){} - which will do nothing but
> cause problems. I’m beginning to think that we should require an
> annotation on the Level declaration of the extension class to avoid that.
>
> Ralph
>
> On Jan 25, 2014, at 9:13 PM, Remko Popma <remko.po...@gmail.com> wrote:
>
> Ralph,
> I copied Nick's code _as is_ and had no compile errors.
> The class is abstract, but instances are defined in the static block as:
> OFF = new Level("OFF", 0) {}; // note the {}: this creates an anonymous
> concrete subclass
>
> I agree that read access needs to be synchronized as well, not just write
> access (the constructor).
> I experimented with several options:
> * synchronizing on plain Object in both constructor and when accessing the
> static Map(s)
> * a ReentrantReadWriteLock
> * a lock-free implementation
>
> I decided against ReentrantReadWriteLock as it has more overhead than
> plain synchronized access and the write access (in the constructors) is
> going to be extremely rare: not worth paying the overhead in the more
> common reads. It is also cumbersome to code.
>
> The lock-free implementation uses an AtomicInteger for the ordinals, and
> an AtomicReference for the Map<String, Level>.
> In the constructor, create a new Map<String, Level> instance based on the
> old copy, add the new instance, and try to call compareAndSet to replace
> the old instance with the new instance. Retry on failure.
>
> Finally, simply synchronizing on the constructorLock object in the
> Level.toLevel() and Level.values() methods may be simplest.
>
> Which of the last two is best depends on how often the toLevel() and
> values() levels are called.
> It turns out they are only called during reconfiguration, so no real need
> to optimize these methods.
> I would argue that simple synchronization may be best in this case.
>
> Remko
>
>
>
> On Sun, Jan 26, 2014 at 1:49 PM, Ralph Goers
> <ralph.go...@dslextreme.com>wrote:
>
> As I am working on this I just want to point out a number of issues with
> the code below:
>
> 1. The class is abstract. The static block is doing a bunch of new Level()
> invocations which obviously generate compile errors on an abstract class.
> I had to make it be a non-abstract class.
> 2. As I pointed out before there is no way to access the “standard” levels
> as an enum. I have addressed that.
> 3. Although the constructor is synchronized access to the Map is not.
> Trying to get from the map while a Level is being added will result in a
> ConcurrentModificationException. I
>
>
