Ist meine static Helper Class Thread save?

Diskutiere Ist meine static Helper Class Thread save? im Allgemeine Java-Themen Bereich.

Bitte aktiviere JavaScript!
Thallius

Thallius

Hi,

ich habe mir eine kleine Klasse geschrieben, die debug informationen in eine Datei schreibt. Diese habe ich statisch gemacht um einfacher von allen anderen Klassen darauf zugreifen zu können. Ich bin jetzt aber nicht sicher, ob diese wirklich Thread safe ist, da sie von vielen Threads gleichzeitig aufgerufen werden kann.
Eigentlich sollte sie das sein da sie nur lokale Variablen (Stackvariablen) ändert. Aber was passiert, wenn die write Methode von mehreren Threads gleichzeitig aufgerunfen wird? Kann es dann zu Problemen kommen ?

Code:
public final class Logger
{
    private static String filename = null;
    private static boolean verbose;

    /**
     * Init logging by creating file for actual day
     *
     * @param verb If true, all logger outputs are also streamed to the console.
     * @return True on success, false on error
     */
    public static boolean init(boolean verb)
    {
        verbose = verb;

        String filePath = GlobalSettings.getPathForFile("logs");
        if(filePath == null)
            return false;

        File directory = new File(filePath);
        if (!directory.exists())
        {
            if(!directory.mkdir())
                return false;

        }
        filename = GlobalSettings.getPathForFile("logs"+File.separator+ Util.sqlDateFromLocalDate(LocalDate.now())+".txt");

        String today = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
        filename = filePath+File.separator+today+".txt";
        return true;
    }

    /**
     * Write text to log
     *
     * @param string Text to write
     */
    public static void log(String string)
    {
        Logger.write(string);
    }

    /**
     * Write double to log
     *
     * @param value Double value to write to log
     */
    @SuppressWarnings("unused")
    public static void log(double value)
    {
        Logger.write(String.format("%f", value)); //$NON-NLS-1$
    }

    /**
     * Write object to log
     *
     * @param object Object to write to log
     */
    public static void log(Object object)
    {
        Logger.write(object.toString());
    }

    /**
     * Write to log
     *
     * @param string Text to write
     */
    private static void write(String string)
    {
        try
        {
            DateFormat dateFormat = new SimpleDateFormat("yyyyMMdd HH:mm:ss"); //$NON-NLS-1$
            Calendar cal = Calendar.getInstance();
            string=dateFormat.format(cal.getTime())+" "+string; //$NON-NLS-1$
            FileWriter fw = new FileWriter(filename,true);
            fw.write(string+System.getProperty("line.separator")); //$NON-NLS-1$
            fw.close();
            if(verbose)
                System.out.println(string);
        }
        catch(IOException ioe)
        {
            System.err.println("IOException: " + ioe.getMessage()); //$NON-NLS-1$
        }
    }
}
 
L

LimDul

Mit ziemlicher Sicherheit nein.

Folgender Block alleine:
Java:
 FileWriter fw = new FileWriter(filename,true);
            fw.write(string+System.getProperty("line.separator")); //$NON-NLS-1$
Was ist, wenn zwei oder mehrere Filewriter gleichzeitig geöffnet werden? Ich glaube nicht, dass das gut geht.

Und wenn ich typische Logging Szenarien denke, halte ich es für extrem viel Overhead jedes mal die Datei auf & zu zu machen.

Außerdem wäre ein ty-with-resources sinnvoll - im Falle einer Exception wird der Filewriter nicht geschlossen.
 
L

LimDul

Noch ein paar Nachträge, die mir eingefallen sind:
- Die Init Methode ist auch nicht Thread-Safe, die kann mit false abbrechen wenn zwei Threads parallel abarbeiten (Verzeichnis ist nicht da - beide Threads versuchen es anzulegen, nur einer gewinnt)
- Warum nutzt du nicht bestehende Logging Frameworks wie z.B. SL4J mit logback?
- An die Log Methode wird normalerweise die Anforderung gestellt, dass sie extrem schnell wieder zurückkommen soll. Das aufmachen eines Filewriters inkl. Schreiben in eine Datei dürfte so ziemlich das Gegenteil von extrem schnell sein. Ich weiß nicht wie Logging Frameworks das machen - aber wenn ich einen Logger schreiben würde, der Daten in externe Resourcen wie Dateien oder Netzwerk schiebt muss das möglichst robust sein ohne die eigentliche Anwendung zu belasten. Sprich, ich würde das wegschreiben ihn einen eigenen Thread auslagern und den Logger die Daten an den anderen Thread übergeben lasse über eine "klassische" Queue die entsprechend Multi-Threading fähig. Der Thread zum protokollieren in die Datei sollte dann sinnvollerweise auch möglichst failsafe sein und ggf. versuchen es noch mal wegzuschreiben bei IO-Exceptions. Ein Logging Framework, was Log Messages verliert weil gerade mal eine IO Resource "Schluckauf" hat, ist kein gutes Logging Framework.

Musst halt für dich entscheiden, wie viel du davon umsetzen willst.
 
Thallius

Thallius

Die Init Methode wird ja nicht von jedem Thread aufgerufen sondern nur einmal beim Start der App.

Geschwindigkeit ist hier auch kein thema da nur Fehler geloggt werden, von denen es hoffentlich nicht viele gibt :)

Ein dickes Framework dafür reinzusetzen widerstrebt mir eben
 
Blender3D

Blender3D

Aber was passiert, wenn die write Methode von mehreren Threads gleichzeitig aufgerunfen wird? Kann es dann zu Problemen kommen ?
Ich denke ja.
Die Strings in einen Buffer geben und von einem Thread das Schreiben ausführen lassen. --> Nur der Logger schreibt in die Datei. Die Aufrufer schreiben in den Buffer.
Habe eine Codevorschlag. Ist nicht getestet. Vielleicht kannst Du daraus etwas machen.
Java:
public final class Logger {
    private volatile static AtomicBoolean busy = new AtomicBoolean(false); // true -> writing is not done yet
    private static String filename = null;
    private static boolean verbose;
    private static volatile ConcurrentLinkedQueue<String> buffer = new ConcurrentLinkedQueue<String>();
    private static boolean running;
    protected static volatile Thread logThread = createLogWriterThread();

    private static Thread createLogWriterThread() {
        running = true;
        return new Thread(new Runnable() {
            @Override
            public void run() {
                while (running) {
                    try {
                        if (!buffer.isEmpty())
                            write();
                        Thread.sleep(5);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        });
    }

    /**
     * Init logging by creating file for actual day
     *
     * @param verb
     *            If true, all logger outputs are also streamed to the console.
     * @return True on success, false on error
     */
    public static boolean init(boolean verb) {
        verbose = verb;

        String filePath = GlobalSettings.getPathForFile("logs");
        if (filePath == null)
            return false;

        File directory = new File(filePath);
        if (!directory.exists()) {
            if (!directory.mkdir())
                return false;
        }
        // 1) assignment to filename
        filename = GlobalSettings
                .getPathForFile("logs" + File.separator + Util.sqlDateFromLocalDate(LocalDate.now()) + ".txt");

        String today = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
        // 2) assignment to filename but not used till now ??
        filename = filePath + File.separator + today + ".txt";
        start();
        return true;
    }

    /**
     * Write text to log
     *
     * @param string
     *            Text to write
     */
    public static void log(String string) {
        Logger.write(string);
    }

    /**
     * Write double to log
     *
     * @param value
     *            Double value to write to log
     */
    @SuppressWarnings("unused")
    public static void log(double value) {
        Logger.write(String.format("%f", value)); //$NON-NLS-1$
    }

    /**
     * Write object to log
     *
     * @param object
     *            Object to write to log
     */
    public static void log(Object object) {
        Logger.write(object.toString());
    }

    private static void start() {
        if (logThread != null && logThread.isAlive())
            return;
        logThread = createLogWriterThread();
        logThread.start();
    }

    public static void stopLogger() {
        running = false;
        logThread = null;
    }

    private static void write(String string) {
        DateFormat dateFormat = new SimpleDateFormat("yyyyMMdd HH:mm:ss"); //$NON-NLS-1$
        Calendar cal = Calendar.getInstance();
        string = dateFormat.format(cal.getTime()) + " " + string; //$NON-NLS-1$
        buffer.offer(string);
    }

    /**
     * Tries to write log with feedback.
     *
     * @param string
     *            Text to write
     * @return true if successful.
     */
    private static void write() {
        if (busy.get())
            return;
        busy.set(true);
        PrintWriter out = null;
        try {
            while (!buffer.isEmpty()) {
                String string = buffer.poll();
                out = new PrintWriter(new BufferedWriter(new FileWriter(filename, true)));
                out.write(string + System.getProperty("line.separator")); //$NON-NLS-1$
                out.close();
                if (verbose)
                    System.out.println(string);
            }
        } catch (IOException e) {
            e.printStackTrace();
            return;
        } finally { // ensure that file will be closed
            if (out != null)
                out.close();
            busy.set(false);
        }
    }
}
 
Blender3D

Blender3D

Java:
      out = new PrintWriter(new BufferedWriter(new FileWriter(filename, true)));
      while (!buffer.isEmpty()) {
                String string = buffer.poll();               
                out.write(string + System.getProperty("line.separator")); //$NON-NLS-1$               
                if (verbose)
                    System.out.println(string);
            }
Writer erzeugen muss noch aus der Schleife raus
 
Thallius

Thallius

Das einzige was mich daran stört ist das polling. Aber Java stellt glaube ich sonst keine Bordmittel zur Verfügung um messaging zu machen
 
Thema: 

Ist meine static Helper Class Thread save?

Passende Stellenanzeigen aus deiner Region:
Anzeige

Neue Themen

Anzeige

Anzeige
Oben