https://issues.dlang.org/show_bug.cgi?id=16232

Robert Schadek <rburners@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rburners@gmail.com

--- Comment #1 from Robert Schadek <rburners@gmail.com> ---
There is nothing we can do about user defined code

--

https://issues.dlang.org/show_bug.cgi?id=16232

--- Comment #2 from ag0aep6g@gmail.com ---
(In reply to Robert Schadek from comment #1)
> There is nothing we can do about user defined code

We have to accommodate for it. sharedLog suggests that it takes care of thread safety, but it doesn't. Either is has to actually make things safe, or it shouldn't suggest that it does. The latter would mean letting the user cast to/from shared.

--

https://issues.dlang.org/show_bug.cgi?id=16232

--- Comment #3 from Robert Schadek <rburners@gmail.com> ---
I will add a comment to make that clear

--

https://issues.dlang.org/show_bug.cgi?id=16232

--- Comment #4 from github-bugzilla@puremagic.com ---
Commit pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/7d7ce4a5ebaca7155e754b1bf7b45366e87d0194 Logger sharedLog comment on thread-safety

fix Issue 16232 - std.experimental.logger.core.sharedLog isn't thread-safe

--

https://issues.dlang.org/show_bug.cgi?id=16232

--- Comment #5 from github-bugzilla@puremagic.com ---
Commit pushed to stable at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/7d7ce4a5ebaca7155e754b1bf7b45366e87d0194 Logger sharedLog comment on thread-safety

--

https://issues.dlang.org/show_bug.cgi?id=16232

ag0aep6g@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #6 from ag0aep6g@gmail.com ---
(In reply to Robert Schadek from comment #3)
> I will add a comment to make that clear

Reopening. I think a comment isn't enough. sharedLog is marked as @safe, but it effectively casts shared away, which isn't safe. It can lead to memory corruption when shared data can be accessed as unshared.

I see two ways out:
1) Make sharedLog @system.
2) Return a shared Logger.

Lengthy example of @safe violation with custom Logger:

----
import std.experimental.logger.core: Logger, LogLevel, sharedLog;

class MyLogger : Logger
{
    ulong* p;

    this() @safe
    {
        super(LogLevel.all);

        /* Allocate a ulong that disrespects cache line boundaries (64 bytes),
        so that it won't be loaded/stored atomically. */
        align(64) static struct S
        {
            align(1):
            ubyte[60] off;
            ulong x = 0;
        }
        auto s = new S;
        this.p = &s.x;
        assert((cast(size_t) p) % 64 == 60);
        assert((cast(size_t) p) % s.x.sizeof == 4);
    }

    override void writeLogMsg(ref LogEntry payload) @safe { assert(false); }
        /* never called */
}

MyLogger sharedMyLogger() @safe
{
    Logger logger = sharedLog();
    return cast(MyLogger) logger;
        /* This is a simple downcast. Not casting away shared. */
}

enum n = 1_000_000;

/* Toggle *p between 0 and ulong.max (n times). */
void write(ulong* p) @safe
{
    foreach (i; 0 .. n) *p = ~*p; /* non-atomic load and store */
}

/* Assert that *p is either 0 or ulong.max (n times). */
void read(ulong* p) @safe
{
    import std.conv: to;

    foreach (i; 0 .. n)
    {
        ulong val = *p; /* non-atomic load */
        assert(val == 0 || val == ulong.max, val.to!string(16)); /* fails */
    }
}

void main()
{
    sharedLog = new MyLogger;

    /* Read and write concurrently. `read` will see a partially written value.
    I.e., memory corruption. */
    import core.thread: Thread;
    new Thread(() @safe { write(sharedMyLogger.p); }).start();
    read(sharedMyLogger.p);
}
----

--

https://issues.dlang.org/show_bug.cgi?id=16232

--- Comment #7 from github-bugzilla@puremagic.com ---
Commit pushed to dmd-cxx at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/7d7ce4a5ebaca7155e754b1bf7b45366e87d0194 Logger sharedLog comment on thread-safety

--

https://issues.dlang.org/show_bug.cgi?id=16232

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--

https://issues.dlang.org/show_bug.cgi?id=16232

ag0aep6g@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--

https://issues.dlang.org/show_bug.cgi?id=16232

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #8 from Dlang Bot <dlang-bot@dlang.rocks> ---
@burner created dlang/phobos pull request #8489 "sharedLog returning shared(Logger)" fixing this issue:

- sharedLog returning shared(Logger)

  Fixes #16232

  changelog

https://github.com/dlang/phobos/pull/8489

--