Singleton

Puppets:

Essa parte que eu não entendi:

public static SingletonLog getInstance() 
    {
        return SingletonLogHolder.instance;
    }

    //Classe auxiliar para criacao da instancia. Evita problemas de sincronizacao de threads.
    private static class SingletonLogHolder 
    {
           private static SingletonLog instance = new SingletonLog();
    }

Toda vez que é chamado o método getInstance() é acriada uma nova instância para a classe SingletonLog,
fiz um teste mesmo criando uma nova instância parace que ela aponta para mesma area de memória, pelo que sei todo
vez que se da um new é alocada uma nova área de memória pois estamos criando um novo “Objeto”, neste caso mesmo dando um new
ele aponta para mesma área de mémoria alguém sabe o porque…?

Hum… apontar sempre o mesmo objeto é o objetivo do padrão singleton. O padrão singleton é usado para garantir que existirá um e um só objeto daquela classe na memoria. Se é isso que está acontecendo está perfeito.
Quando ao codigo acima serve para esconder a instancia do singleton. É uma forma extra de protecção para o modelo comum que é usar uma varável estatica da propria classe. Isso é porque algums maniacos do reflection conseguem obter a instancia directamente mesmo quando quando ela é private. Desta outra forma é mais dificil.

Ué… só é dado new uma vez… na hora que a classe SingletonLogHolder é carregada e suas variáveis são inicializadas.

Por que você acha que tem mais de um new?

É criado uma única instância. Ela é acessada sempre que se chama o getInstance(). Só não entendi como essa forma do código ajuda na sincronização de Threads...

Em geral, eu implemento diferente:

public class Teste
{
  private Teste instancia;

  private Teste(){}

  public Teste getInstancia()
  {
     if(instancia==null)
     {
        instancia = new Teste();
     }

     return instancia;
  }
}

Ou assim

public class Teste
{
  private Teste instancia = new Teste();

  private Teste(){}

  public Teste getInstancia()
  {
     return instancia;
  }
}

Alguém poderia me dizer o que há de errado no meu código para Singleton?

eclipso:

Alguém poderia me dizer o que há de errado no meu código para Singleton?

Nada. Como eu disse antes, aquela outra forma de fazer é mais segura contra acesso via reflection.
Hoje em dia é considerada a melhor forma de implementar singleton.

O que isso tem a haver com threads ? Nada. Se o seu objeto for acessado por várias threads tanto faz como ele é criado. O ponto é se ele não existe quando uma thread chama getInstance pela primeira vez. Ai o seu if pode provocar a criação de dois objetos e isso viola o padrão. Como a a forma em que vc dá o new diractamente no atributo isso não é um problema.

Há a necessidade de criar aquela Inner Class ali?
Não vi vantagem nenhuma.

É…Acho que aquela classe interna tem o objetivo de proteger de acesso via reflection…

eclipso:

É...Acho que aquela classe interna tem o objetivo de proteger de acesso via reflection...

package testes;

public class Singleton {
    private static class SingletonHolder
    {
        private static Singleton instance = new Singleton();
    }
    
    private Singleton()
    {        
    }
    
    public Singleton getInstance()
    {
        return SingletonHolder.instance;
    }
}

package testes;

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;

public class Main {
    public static void main(String[] args) {        
        try
        {
            //Pegamos a inner class
            Class<?> singletonHolder = Singleton.class.getDeclaredClasses()[1];            
            //Pegamos seu campo secreto
            Field instance = singletonHolder.getDeclaredField("instance");
            instance.setAccessible(true); //Tornamos ele acessível
            
            //Pegamos o construtor do singleton
            Constructor c = Singleton.class.getDeclaredConstructors()[0];
            c.setAccessible(true); //tornamos ele acessível
            
            //Voilá... 2 instâncias.
            instance.set(null, c.newInstance());
        } catch (Exception e)
        {
            e.printStackTrace();
        }
    }
}

Isso prova que não só é possível criar uma segunda instância do Singleton com a inner class, como também é possível alterar o valor da variável estática dentro dela.
Alguém tem alguma idéia do por que usar a inner class?

Aliás, nem precisava ir tão longe assim. Só com as linhas do setAcessible no construtor do Singleton já me permitiria criar várias instâncias. Via de regra, não podemos falar em padrões, qualidade de código, etc... se levarmos reflexão em conta. Afinal, ela permite acabar com o encapsulamento. Pretty nasty, huh?

ViniGodoy:

Via de regra, não podemos falar em padrões, qualidade de código, etc… se levarmos reflexão em conta. Afinal, ela permite acabar com o encapsulamento. Pretty nasty, huh?

Ele não acaba com encapsulamento. Encapsulamento não é sobre esconder dados e sim sobre o que você disponibiliza aos seus clientes. Se o cliente ignora o que é disponibilizado e entra na classe para pegar o que quer o problema não é da classe e sim de quem desenhou o cliente.

De qualquer forma o comentário “Evita problemas de sincronizacao de threads.”, não tem lá muita coisa haver.

Para satisfazer a questão da thread, o getInstance deveria ser:

public static synchronized SingletonLog getInstance(){
 ...
}

Lembrando que não é só reflexão que “estraga” um Singleton. Coloque uma classe com esta em cluster e tente garantir uma única instancia em VMs separadas …

Por isso eu falei “se levarmos reflexão em conta”. A reflexão é uma maneira explícita do cliente por sua própria conta e risco quebrar o encapsulamento. E não estou falando só de informações ocultas, mas de métodos e lógica também.

Eu entendi o que você quis dizer mas é importante desassociar encapsulamento de private.

ViniGodoy:

Isso prova que não só é possível criar uma segunda instância do Singleton com a inner class, como também é possível alterar o valor da variável estática dentro dela.
Alguém tem alguma idéia do por que usar a inner class?

Aliás, nem precisava ir tão longe assim. Só com as linhas do setAcessible no construtor do Singleton já me permitiria criar várias instâncias. Via de regra, não podemos falar em padrões, qualidade de código, etc… se levarmos reflexão em conta. Afinal, ela permite acabar com o encapsulamento. Pretty nasty, huh?

Sim. Mas vc fez algums passos que nem sempre seriam aceites pelo ambiente como setAcessible(true). Se um SecurityManager estiver presente essa instrução dá erro evitando que vc acess o interior da classe. O argumento para a solução de usar inner classe para singleton é que coloca mais uma dificultadade à reflection. Isto porque quem faz o reflection não sabe o interior da classe ( no caso vc sabe e por isso foi facil chamar tudo direitinho). A implementação mais usual com um atributo estático é mais facil de burlar na opiniõ das pessoas que defendem o uso de inner class. Isto já foi discutido aqui faz algum tempo devido a um artigo que defendia que singletons em java não são singletons verdadeiros porque podem ser burlados via reflection.

O meu argumento na altura foi o mesmo que o seu: Não podemos levar reflection em conta quando desenhamos implementações de padrões. Contudo, a forma com inner classe é a que é defendida hoje em dia pelo “main stream” embora, como vc mesmo provou é tão inutil em esconder as coisas como a antiga.

Náo só por reflexão. Como coloquei, se o método de acesso não tiver um synchronized vc pode ter mais de uma instancia via acessos concorrentes. Se a aplicação rodar em cluster, tbm não tem como garantir uma única instancia.

Náo só por reflexão. Como coloquei, se o método de acesso não tiver um synchronized vc pode ter mais de uma instancia via acessos concorrentes. Se a aplicação rodar em cluster, tbm não tem como garantir uma única instancia.

Sim, com certeza.
99,99% das vezes quando a palavra “singleton” é usada no contexto de java a pessoa não se refere ao padrão Singleton tal qual definido nos catalogos de padrões (classe que só permite uma instancia), mas sim ao padrão Shared Object ( objeto global partilhado por todos ). Em cluster não faz sequer sentido pensar em singleton, enquanto Shared Objet cai que nem uma luva.
O problema do syncronized é mais especifico do lazy loading e não tanto do padrão singleton. É que normalmente vemos implementações de singleton com lazy loading, mas que em 99,99% dos casos não serve para nada pois o objeto é na realidade um Shared Object e não tem como correr a aplicação sem iniciar o objeto. O lazy loading causa esses problemas mesmo em outras aplicações como por exemplo em relações entre entidades num modelo de dominio gerenciado.

sergiotaborda:

Sim. Mas vc fez algums passos que nem sempre seriam aceites pelo ambiente como setAcessible(true). Se um SecurityManager estiver presente essa instrução dá erro evitando que vc acess o interior da classe. O argumento para a solução de usar inner classe para singleton é que coloca mais uma dificultadade à reflection. Isto porque quem faz o reflection não sabe o interior da classe ( no caso vc sabe e por isso foi facil chamar tudo direitinho). A implementação mais usual com um atributo estático é mais facil de burlar na opiniõ das pessoas que defendem o uso de inner class. Isto já foi discutido aqui faz algum tempo devido a um artigo que defendia que singletons em java não são singletons verdadeiros porque podem ser burlados via reflection.

O meu argumento na altura foi o mesmo que o seu: Não podemos levar reflection em conta quando desenhamos implementações de padrões. Contudo, a forma com inner classe é a que é defendida hoje em dia pelo “main stream” embora, como vc mesmo provou é tão inutil em esconder as coisas como a antiga.

Concordo com tudo que vc falou.

Na verdade, se alguém recorreu a reflection para burlar um padrão, ele pode (assim como eu fiz para poder escrever esse exemplo) usar os próprios métodos de reflection para inspecionar a classe. Se partiu para o uso de reflexão, definitivamente é alguém mal intencionado ou, no mínimo, alguém que está disposto a assumir o risco. E definitivamente, é um programador que tem consciência que está usando um recurso perigoso.

O acesso synchronized só é necessário se você fizer lazy loading. Caso contrário, a VM garante a inicialização do atributo estático e, como seu valor não muda no decorrer do programa, não necessita de sincronização (uma das boas garantias da imutabilidade).

Agora, quando se fala em cluster e distribuição, você certamente terá problemas muito maiores do que simplesmente garantir a unicidade de um Singleton. Eu vejo o Singleton como um padrão prático para aplicações simples, que rodem em hardware restrito. Ou para aplicações mais controladas, como jogos single-player. Comercialmente ele é um padrão de pouca valia, já que existem opções muito melhores.

Pois é Sérgio. Acho que nunca ví em um legado uma implementação que não fosse Lazy… e até hoje não sei o pq disso.

Ops, e uma errata quanto a meu primeiro post aqui nesta thread:

… isso tbm explica melhor o que a inner-class esta fazendo ali (claro, se por algum motivo for necessário usar lazy).