Skip to content

string copy elision in StructuredAttrs::parse#11

Closed
NaN-git wants to merge 14 commits intoMic92:mainfrom
NaN-git:fix-lazy-attrs
Closed

string copy elision in StructuredAttrs::parse#11
NaN-git wants to merge 14 commits intoMic92:mainfrom
NaN-git:fix-lazy-attrs

Conversation

@NaN-git
Copy link

@NaN-git NaN-git commented Aug 6, 2025

Motivation

  • formatting fix
  • revert to simplified Derivation::parseDerivation code

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Mic92 and others added 14 commits August 6, 2025 20:24
Replace rand() with atomic counter for unique temp link names and add
retry loop to handle EEXIST errors when multiple processes optimize
the store simultaneously.
Replace rand() with atomic counter and add retry loop to handle
race conditions when creating temporary symlinks for garbage
collector roots.
Add atomic counter to temporary log file names and retry loop to
handle EEXIST errors. If another process creates the log file first,
we gracefully exit since the log already exists.
Handle race conditions in binary cache file creation by retrying
with a new temporary name on EEXIST errors. Uses existing
makeTempPath which already has an atomic counter.
Signed-off-by: Jörg Thalheim <null>
* type hints
* use argparse
* use pathlib
* use TemporaryDirectory
This introduces a new release script for nix-based releases.
Compared to the old version this improves the following aspects:

- fully type checked
- no longer depends on beeing executed on eelco's work station
- no dependency on the third-party libraries compared to the old version
  (aws, LWP, json parser etc.)
- self-test and dry run mode to test releases without having specific
  access to release buckets or docker registry.
  The results of a release can be visually inspected and debugged locally.
- improved error handling and logging
- ci-friendly: all dependencies are managed by nix and locked by the local flake
- no dependency on an external docker daemon by using podman
- more arguments and documented help to publish releases in different buckets/registry
This allows to force colors but not disable other features such as the
progress bar. This is for instance useful in CI environments.

Adapted from
https://git.lix.systems/lix-project/lix/commit/378ec5fb0611e314511a7afc806664205846fc2e
and others.
revert to simplified `Derivation::parseDerivation` code
@Mic92 Mic92 force-pushed the main branch 3 times, most recently from 4a6c4d6 to cdbeb39 Compare August 10, 2025 10:40
Mic92 pushed a commit that referenced this pull request Aug 11, 2025
This was removed in NixOS#11152. However,
we need it for the multi-threaded evaluator, because otherwise Boehm
GC will crash while scanning the thread stack:

  #0  GC_push_all_eager (bottom=<optimized out>, top=<optimized out>) at extra/../mark.c:1488
  #1  0x00007ffff74691d5 in GC_push_all_stack_sections (lo=<optimized out>, hi=<optimized out>, traced_stack_sect=0x0) at extra/../mark_rts.c:704
  #2  GC_push_all_stacks () at extra/../pthread_stop_world.c:876
  #3  GC_default_push_other_roots () at extra/../os_dep.c:2893
  #4  0x00007ffff746235c in GC_mark_some (cold_gc_frame=0x7ffee8ecaa50 "`\304G\367\377\177") at extra/../mark.c:374
  #5  0x00007ffff7465a8d in GC_stopped_mark (stop_func=stop_func@entry=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:875
  #6  0x00007ffff7466724 in GC_try_to_collect_inner (stop_func=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:624
  #7  0x00007ffff7466a22 in GC_collect_or_expand (needed_blocks=needed_blocks@entry=1, ignore_off_page=ignore_off_page@entry=0, retry=retry@entry=0) at extra/../alloc.c:1688
  #8  0x00007ffff746878f in GC_allocobj (gran=<optimized out>, kind=<optimized out>) at extra/../alloc.c:1798
  #9  GC_generic_malloc_inner (lb=<optimized out>, k=k@entry=1) at extra/../malloc.c:193
  #10 0x00007ffff746cd40 in GC_generic_malloc_many (lb=<optimized out>, k=<optimized out>, result=<optimized out>) at extra/../mallocx.c:477
  #11 0x00007ffff746cf35 in GC_malloc_kind (bytes=120, kind=1) at extra/../thread_local_alloc.c:187
  #12 0x00007ffff796ede5 in nix::allocBytes (n=<optimized out>, n=<optimized out>) at ../src/libexpr/include/nix/expr/eval-inline.hh:19

This is because it will use the stack pointer of the coroutine, so it
will scan a region of memory that doesn't exist, e.g.

  Stack for thread 0x7ffea4ff96c0 is [0x7ffe80197af0w,0x7ffea4ffa000)

(where 0x7ffe80197af0w is the sp of the coroutine and 0x7ffea4ffa000
is the base of the thread stack).

We don't scan coroutine stacks, because currently they don't have GC
roots (there is no evaluation happening in coroutines). So there is
currently no need to restore the other parts of the original patch,
such as BoehmGCStackAllocator.
@Mic92 Mic92 force-pushed the main branch 5 times, most recently from c4a53c5 to 788ecb2 Compare August 12, 2025 07:17
@Mic92
Copy link
Owner

Mic92 commented Aug 12, 2025

Applied in 788ecb2 thanks

@Mic92 Mic92 closed this Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants