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

[d-bugmail--- via Digitalmars-d-bugs]

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

Dlang Bot  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #9 from Dlang Bot  ---
dlang/phobos pull request #8489 "sharedLog returning shared(Logger)" was merged
into master:

- 63f49d6687be573b6e6c8a5f7401c5a82ea865b9 by Robert burner Schadek:
  sharedLog returning shared(Logger)

  Fixes #16232

  changelog

  -preview=nosharedaccess

  code review

  working on the tests

  whitespace

  spaces behind casts

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

--

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

[d-bugmail--- via Digitalmars-d-bugs]

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

Dlang Bot  changed:

   What|Removed |Added

   Keywords||pull

--- Comment #8 from Dlang Bot  ---
@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

--

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

[d-bugmail--- via Digitalmars-d-bugs]

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

ag0ae...@gmail.com changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--

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

[d-bugmail--- via Digitalmars-d-bugs]

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

github-bugzi...@puremagic.com changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--

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

[d-bugmail--- via Digitalmars-d-bugs]

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

--- Comment #7 from github-bugzi...@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

--

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

[via Digitalmars-d-bugs]

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

ag0ae...@gmail.com changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #6 from ag0ae...@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 = 
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);
}


--

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

[via Digitalmars-d-bugs]

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

--- Comment #5 from github-bugzi...@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

--

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

[via Digitalmars-d-bugs]

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

--- Comment #4 from github-bugzi...@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

--

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

[via Digitalmars-d-bugs]

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

--- Comment #3 from Robert Schadek  ---
I will add a comment to make that clear

--

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

[via Digitalmars-d-bugs]

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

--- Comment #2 from ag0ae...@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.

--

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

[via Digitalmars-d-bugs]

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

Robert Schadek  changed:

   What|Removed |Added

 CC||rburn...@gmail.com

--- Comment #1 from Robert Schadek  ---
There is nothing we can do about user defined code

--